Skip to content

Commit 30b783e

Browse files
john-hilljohn-hill
authored
Merge pull request #5501 from brucehoff/PLFM-9353
PLFM-9353
2 parents e41eefe + 721efe7 commit 30b783e

File tree

16 files changed

+74
-76
lines changed

16 files changed

+74
-76
lines changed

‎lib/jdomodels/src/test/java/org/sagebionetworks/repo/model/dbo/auth/DBOAuthenticationDAOImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public void testBootstrap() throws Exception {
163163
// Most bootstrapped users should have signed the terms
164164
List<BootstrapPrincipal> ugs = userGroupDAO.getBootstrapPrincipals();
165165
for (BootstrapPrincipal agg: ugs) {
166-
if (agg instanceof BootstrapUser && !AuthorizationUtils.isDefaultRealmAnonymousId(agg.getId())) {
166+
if (agg instanceof BootstrapUser && !BOOTSTRAP_PRINCIPAL.ANONYMOUS_USER.getPrincipalId().equals(agg.getId())) {
167167
MapSqlParameterSource param = new MapSqlParameterSource();
168168
param.addValue("principalId", agg.getId());
169169
basicDAO.getObjectByPrimaryKey(DBOCredential.class, param).get();

‎lib/models/src/main/java/org/sagebionetworks/repo/model/AuthorizationConstants.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,13 @@ public enum ACL_SCHEME{
8080
*/
8181
public static final String USER_ID_PARAM = "userId";
8282

83+
/**
84+
* Request parameter indicating whether the user is anonymous. Note that
85+
* callers of the service do not actually use this parameter. Instead they
86+
* use a token parameter which is then validated to determine if the user is anonymous
87+
*/
88+
public static final String ANONYMOUS_PARAM = "anonymous";
89+
8390
/**
8491
* The name of the client make the REST call. For a few calls, behavior will change depending on domain (at the
8592
* least, email contents change).

‎lib/models/src/main/java/org/sagebionetworks/repo/model/AuthorizationUtils.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,11 @@
11
package org.sagebionetworks.repo.model;
22

3-
import org.sagebionetworks.repo.model.AuthorizationConstants.BOOTSTRAP_PRINCIPAL;
43
import org.sagebionetworks.repo.model.auth.AuthorizationStatus;
54

65
public class AuthorizationUtils {
76

87
private static final String FILE_HANDLE_UNAUTHORIZED_TEMPLATE = "Only the creator of a FileHandle can access it directly by its ID. FileHandleId = '%1$s', UserId = '%2$s'";
98

10-
public static boolean isDefaultRealmAnonymousId(Long id) {
11-
return id == null || BOOTSTRAP_PRINCIPAL.ANONYMOUS_USER.getPrincipalId().equals(id);
12-
}
13-
149
/**
1510
* returns true iff the user is a certified user
1611
*

‎services/repository-managers/src/test/java/org/sagebionetworks/repo/manager/UserManagerImplTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ public void tearDown() throws Exception {
5656
public void testGetAnonymous() throws Exception {
5757
UserInfo ui = userManager.getUserInfo(BOOTSTRAP_PRINCIPAL.ANONYMOUS_USER.getPrincipalId());
5858
assertTrue(ui.isUserAnonymous());
59-
assertTrue(AuthorizationUtils.isDefaultRealmAnonymousId(ui.getId()));
60-
assertTrue(AuthorizationUtils.isDefaultRealmAnonymousId(Long.parseLong(ui.getId().toString())));
59+
assertEquals(BOOTSTRAP_PRINCIPAL.ANONYMOUS_USER.getPrincipalId(), ui.getId());
60+
assertEquals(BOOTSTRAP_PRINCIPAL.ANONYMOUS_USER.getPrincipalId(), Long.parseLong(ui.getId().toString()));
6161
assertNotNull(ui.getId());
6262
assertEquals(2, ui.getGroups().size());
6363
assertTrue(ui.getGroups().contains(ui.getId()));

‎services/repository/src/main/java/org/sagebionetworks/auth/HttpAuthUtil.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,5 +198,10 @@ public static void rejectWithOAuthError(HttpServletResponse resp, OAuthErrorCode
198198
}
199199
reject(resp, er, status);
200200
}
201+
202+
public static boolean isAnonymous(HttpServletRequest httpRequest) {
203+
String param = httpRequest.getParameter(AuthorizationConstants.ANONYMOUS_PARAM);
204+
return Boolean.parseBoolean(param);
205+
}
201206

202207
}

‎services/repository/src/main/java/org/sagebionetworks/auth/filter/AcceptTermsOfUseFilter.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ public class AcceptTermsOfUseFilter implements Filter {
3232

3333
@Autowired
3434
private AuthenticationService authenticationService;
35-
36-
@Autowired
37-
private UserManager userManager;
3835

3936
@Override
4037
public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain)
@@ -58,8 +55,8 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
5855
Long userId = Long.parseLong(userIdParam);
5956

6057
// If the user is not anonymous, check if they have accepted the terms of use
61-
UserInfo userInfo = userManager.getUserInfo(userId);
62-
if (!userInfo.isUserAnonymous()) {
58+
boolean isAnonymous = HttpAuthUtil.isAnonymous(httpRequest);
59+
if (!isAnonymous) {
6360
if (!authenticationService.hasUserAcceptedTermsOfService(userId)) {
6461
HttpAuthUtil.rejectWithErrorResponse(httpResponse, TOU_UNSIGNED_REASON, HttpStatus.FORBIDDEN);
6562
return;

‎services/repository/src/main/java/org/sagebionetworks/auth/filter/AuthenticationFilter.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java.io.IOException;
44
import java.util.HashMap;
55
import java.util.Map;
6+
import java.util.Optional;
67

78
import javax.servlet.Filter;
89
import javax.servlet.FilterChain;
@@ -22,6 +23,7 @@
2223
import org.sagebionetworks.repo.model.AuthenticationMethod;
2324
import org.sagebionetworks.repo.model.AuthorizationConstants;
2425
import org.sagebionetworks.repo.model.AuthorizationConstants.BOOTSTRAP_PRINCIPAL;
26+
import org.sagebionetworks.repo.model.RealmDao;
2527
import org.sagebionetworks.repo.web.ForbiddenException;
2628
import org.sagebionetworks.repo.web.OAuthException;
2729
import org.sagebionetworks.util.ThreadLocalProvider;
@@ -46,6 +48,9 @@ public class AuthenticationFilter implements Filter {
4648

4749
@Autowired
4850
private OpenIDConnectManager oidcManager;
51+
52+
@Autowired
53+
private RealmDao realmDao;
4954

5055
@Override
5156
public void destroy() { }
@@ -68,6 +73,7 @@ public void doFilter(ServletRequest servletRqst, ServletResponse servletResponse
6873
}
6974

7075
Long userId = null;
76+
boolean isAnonymous = false;
7177

7278
if (!isTokenEmptyOrNull(accessToken)) {
7379
try {
@@ -76,6 +82,8 @@ public void doFilter(ServletRequest servletRqst, ServletResponse servletResponse
7682
if (authenticationMethod == null) { // accessToken came in as sessionToken
7783
authenticationMethod = AuthenticationMethod.BEARERTOKEN;
7884
}
85+
Optional<String> anonymousUserRealm = realmDao.getRealmForAnonymousPrincipal(userId.toString());
86+
isAnonymous = anonymousUserRealm.isPresent(); // true if userId is the anonymous user in some realm
7987
} catch (IllegalArgumentException | ForbiddenException | OAuthClientNotVerifiedException e) {
8088
String failureReason = "Invalid access token";
8189
HttpAuthUtil.reject((HttpServletResponse)servletResponse, failureReason);
@@ -88,6 +96,7 @@ public void doFilter(ServletRequest servletRqst, ServletResponse servletResponse
8896
}
8997
} else { // anonymous
9098
userId = BOOTSTRAP_PRINCIPAL.ANONYMOUS_USER.getPrincipalId();
99+
isAnonymous = true;
91100
}
92101

93102
if (authenticationMethod == null && HttpAuthUtil.usesBasicAuthentication(req)) {
@@ -104,6 +113,7 @@ public void doFilter(ServletRequest servletRqst, ServletResponse servletResponse
104113
try {
105114
Map<String, String[]> modParams = new HashMap<String, String[]>(req.getParameterMap());
106115
modParams.put(AuthorizationConstants.USER_ID_PARAM, new String[] { userId.toString() });
116+
modParams.put(AuthorizationConstants.ANONYMOUS_PARAM, new String[] { ""+isAnonymous });
107117
Map<String, String[]> modHeaders = HttpAuthUtil.filterAuthorizationHeaders(req);
108118
if (accessToken!=null) {
109119
HttpAuthUtil.setBearerTokenHeader(modHeaders, accessToken);

‎services/repository/src/main/java/org/sagebionetworks/auth/filter/TwoFactorAuthRequiredFilter.java‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
import javax.servlet.http.HttpServletRequest;
99
import javax.servlet.http.HttpServletResponse;
1010

11+
import org.sagebionetworks.auth.HttpAuthUtil;
1112
import org.sagebionetworks.repo.manager.feature.FeatureManager;
1213
import org.sagebionetworks.repo.model.AuthorizationConstants;
1314
import org.sagebionetworks.repo.model.AuthorizationConstants.BOOTSTRAP_PRINCIPAL;
1415
import org.sagebionetworks.repo.model.GroupMembersDAO;
15-
import org.sagebionetworks.repo.model.RealmDao;
1616
import org.sagebionetworks.repo.model.auth.AuthenticationDAO;
1717
import org.sagebionetworks.repo.model.feature.Feature;
1818
import org.sagebionetworks.repo.web.TwoFactorAuthEnabledRequiredException;
@@ -28,14 +28,12 @@ public class TwoFactorAuthRequiredFilter extends OncePerRequestFilter {
2828
private GroupMembersDAO groupMemberDao;
2929
private AuthenticationDAO authDao;
3030
private FeatureManager featureManager;
31-
private RealmDao realmDao;
3231

3332

34-
public TwoFactorAuthRequiredFilter(GroupMembersDAO groupMemberDao, AuthenticationDAO authDao, FeatureManager featureManager, RealmDao realmDao) {
33+
public TwoFactorAuthRequiredFilter(GroupMembersDAO groupMemberDao, AuthenticationDAO authDao, FeatureManager featureManager) {
3534
this.groupMemberDao = groupMemberDao;
3635
this.authDao = authDao;
3736
this.featureManager = featureManager;
38-
this.realmDao = realmDao;
3937
}
4038

4139
@Override
@@ -46,7 +44,7 @@ protected void doFilterInternal(HttpServletRequest httpRequest, HttpServletRespo
4644

4745
// The anonymous user is not a conventional user and cannot enable 2fa
4846
// we check to see if the given user is the anonymous user in any realm
49-
if (realmDao.getRealmForAnonymousPrincipal(userId.toString()).isPresent()) {
47+
if (HttpAuthUtil.isAnonymous(httpRequest)) {
5048
filterChain.doFilter(httpRequest, httpResponse);
5149
return;
5250
}

‎services/repository/src/main/java/org/sagebionetworks/repo/web/OAuthScopeInterceptor.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.sagebionetworks.repo.manager.oauth.ClaimsJsonUtil;
1717
import org.sagebionetworks.repo.manager.oauth.OIDCTokenManager;
1818
import org.sagebionetworks.repo.model.AuthorizationConstants;
19-
import org.sagebionetworks.repo.model.AuthorizationUtils;
2019
import org.sagebionetworks.repo.model.oauth.OAuthScope;
2120
import org.springframework.beans.factory.annotation.Autowired;
2221
import org.springframework.core.MethodParameter;
@@ -59,15 +58,6 @@ public static boolean hasUserIdParameterOrAccessTokenHeader(HandlerMethod handle
5958
return false;
6059
}
6160

62-
public static boolean isUnAuthenticated(HttpServletRequest request) {
63-
// if a request is unauthenticated then AuthenticationFilter will fill in
64-
// user-id with the id of the default anonymous user
65-
String userIdRequestParameter = request.getParameter(AuthorizationConstants.USER_ID_PARAM);
66-
67-
return userIdRequestParameter==null ||
68-
AuthorizationUtils.isDefaultRealmAnonymousId(Long.parseLong(userIdRequestParameter));
69-
}
70-
7161
public static boolean isServiceCall(HttpServletRequest request) {
7262
String serviceName = request.getHeader(AuthorizationConstants.SYNAPSE_HEADER_SERVICE_NAME);
7363
return serviceName != null;
@@ -83,7 +73,7 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons
8373
}
8474

8575
// unauthenticated requests can't have scope checked, as there is no access token defining scopes
86-
if (isUnAuthenticated(request)) {
76+
if (HttpAuthUtil.isAnonymous(request)) {
8777
return true;
8878
}
8979

‎services/repository/src/main/java/org/sagebionetworks/repo/web/filter/throttle/RequestThrottleFilter.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@
1111
import javax.servlet.ServletException;
1212
import javax.servlet.ServletRequest;
1313
import javax.servlet.ServletResponse;
14+
import javax.servlet.http.HttpServletRequest;
1415

16+
import org.sagebionetworks.auth.HttpAuthUtil;
1517
import org.sagebionetworks.cloudwatch.Consumer;
16-
import org.sagebionetworks.repo.model.AuthorizationUtils;
1718
import org.sagebionetworks.repo.web.HttpRequestIdentifier;
1819
import org.sagebionetworks.repo.web.HttpRequestIdentifierUtils;
1920
import org.springframework.beans.factory.annotation.Autowired;
@@ -34,7 +35,7 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
3435
HttpRequestIdentifier httpRequestIdentifier = HttpRequestIdentifierUtils.getRequestIdentifier(request);
3536

3637
//TODO: remove exception for anonymous users once java client has a way to get session id from cookies
37-
if (isMigrationAdmin(httpRequestIdentifier.getUserId()) || AuthorizationUtils.isDefaultRealmAnonymousId(httpRequestIdentifier.getUserId())) { //do not throttle the admin responsible for migration.
38+
if (isMigrationAdmin(httpRequestIdentifier.getUserId()) || HttpAuthUtil.isAnonymous((HttpServletRequest)request)) { //do not throttle the admin responsible for migration.
3839
//proceed to next filter and exit early
3940
chain.doFilter(request, response);
4041
return;

0 commit comments

Comments
 (0)