Skip to content

Commit 86bb6d0

Browse files
committed
clean up use of session for spa
1 parent f79a02b commit 86bb6d0

1 file changed

Lines changed: 36 additions & 103 deletions

File tree

src/main/java/edu/harvard/iq/dataverse/api/Access.java

Lines changed: 36 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import edu.harvard.iq.dataverse.authorization.Permission;
1616
import edu.harvard.iq.dataverse.authorization.RoleAssignee;
1717
import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
18-
import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser;
1918
import edu.harvard.iq.dataverse.authorization.users.GuestUser;
2019
import edu.harvard.iq.dataverse.authorization.users.User;
2120
import edu.harvard.iq.dataverse.dataaccess.DataAccess;
@@ -185,14 +184,12 @@ public BundleDownloadInstance datafileBundle(@Context ContainerRequestContext cr
185184

186185
DataFile df = findDataFileOrDieWrapper(fileId);
187186

188-
// This will throw a ForbiddenException if access isn't authorized:
189-
checkAuthorization(getRequestUser(crc), df);
187+
// This will throw a ForbiddenException if access isn't authorized:
188+
checkAuthorization(crc, df);
190189

191190
if (gbrecs != true && df.isReleased()){
192191
// Write Guestbook record if not done previously and file is released
193-
//This calls findUserOrDie which will retrieve the key param or api token header, or the workflow token header.
194-
User apiTokenUser = findAPITokenUser(getRequestUser(crc));
195-
gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, apiTokenUser);
192+
gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, getUser(crc));
196193
guestbookResponseService.save(gbr);
197194
MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, df);
198195
mdcLogService.logEntry(entry);
@@ -283,13 +280,12 @@ public Response datafile(@Context ContainerRequestContext crc, @PathParam("fileI
283280
// (nobody should ever be using this API on a harvested DataFile)!
284281
}
285282

286-
// This will throw a ForbiddenException if access isn't authorized:
287-
checkAuthorization(getRequestUser(crc), df);
283+
// This will throw a ForbiddenException if access isn't authorized:
284+
checkAuthorization(crc, df);
288285

289286
if (gbrecs != true && df.isReleased()){
290287
// Write Guestbook record if not done previously and file is released
291-
User apiTokenUser = findAPITokenUser(getRequestUser(crc));
292-
gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, apiTokenUser);
288+
gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, getUser(crc));
293289
}
294290

295291
DownloadInfo dInfo = new DownloadInfo(df);
@@ -624,7 +620,7 @@ public DownloadInstance downloadAuxiliaryFile(@Context ContainerRequestContext c
624620
// as defined for the DataFile itself), and will throw a ForbiddenException
625621
// if access is denied:
626622
if (!publiclyAvailable) {
627-
checkAuthorization(getRequestUser(crc), df);
623+
checkAuthorization(crc, df);
628624
}
629625

630626
return downloadInstance;
@@ -644,7 +640,7 @@ public DownloadInstance downloadAuxiliaryFile(@Context ContainerRequestContext c
644640
public Response postDownloadDatafiles(@Context ContainerRequestContext crc, String fileIds, @QueryParam("gbrecs") boolean gbrecs, @Context UriInfo uriInfo, @Context HttpHeaders headers, @Context HttpServletResponse response) throws WebApplicationException {
645641

646642

647-
return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, null);
643+
return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, null);
648644
}
649645

650646
@GET
@@ -665,7 +661,7 @@ public Response downloadAllFromLatest(@Context ContainerRequestContext crc, @Pat
665661
// We don't want downloads from Draft versions to be counted,
666662
// so we are setting the gbrecs (aka "do not write guestbook response")
667663
// variable accordingly:
668-
return downloadDatafiles(getRequestUser(crc), fileIds, true, uriInfo, headers, response, "draft");
664+
return downloadDatafiles(crc, fileIds, true, uriInfo, headers, response, "draft");
669665
}
670666
}
671667

@@ -686,7 +682,7 @@ public Response downloadAllFromLatest(@Context ContainerRequestContext crc, @Pat
686682
}
687683

688684
String fileIds = getFileIdsAsCommaSeparated(latest.getFileMetadatas());
689-
return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, latest.getFriendlyVersionNumber());
685+
return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, latest.getFriendlyVersionNumber());
690686
} catch (WrappedResponse wr) {
691687
return wr.getResponse();
692688
}
@@ -736,7 +732,7 @@ public Command<DatasetVersion> handleLatestPublished() {
736732
if (dsv.isDraft()) {
737733
gbrecs = true;
738734
}
739-
return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, dsv.getFriendlyVersionNumber().toLowerCase());
735+
return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, dsv.getFriendlyVersionNumber().toLowerCase());
740736
} catch (WrappedResponse wr) {
741737
return wr.getResponse();
742738
}
@@ -777,10 +773,10 @@ private String generateMultiFileBundleName(Dataset dataset, String versionTag) {
777773
@Path("datafiles/{fileIds}")
778774
@Produces({"application/zip"})
779775
public Response datafiles(@Context ContainerRequestContext crc, @PathParam("fileIds") String fileIds, @QueryParam("gbrecs") boolean gbrecs, @Context UriInfo uriInfo, @Context HttpHeaders headers, @Context HttpServletResponse response) throws WebApplicationException {
780-
return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, null);
776+
return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, null);
781777
}
782778

783-
private Response downloadDatafiles(User user, String rawFileIds, boolean donotwriteGBResponse, UriInfo uriInfo, HttpHeaders headers, HttpServletResponse response, String versionTag) throws WebApplicationException /* throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {
779+
private Response downloadDatafiles(ContainerRequestContext crc, String rawFileIds, boolean donotwriteGBResponse, UriInfo uriInfo, HttpHeaders headers, HttpServletResponse response, String versionTag) throws WebApplicationException /* throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {
784780
final long zipDownloadSizeLimit = systemConfig.getZipDownloadLimit();
785781

786782
logger.fine("setting zip download size limit to " + zipDownloadSizeLimit + " bytes.");
@@ -801,8 +797,8 @@ private Response downloadDatafiles(User user, String rawFileIds, boolean donotwr
801797

802798
String customZipServiceUrl = settingsService.getValueForKey(SettingsServiceBean.Key.CustomZipDownloadServiceUrl);
803799
boolean useCustomZipService = customZipServiceUrl != null;
804-
805-
User apiTokenUser = findAPITokenUser(user); //for use in adding gb records if necessary
800+
801+
User user = getUser(crc);
806802

807803
Boolean getOrig = false;
808804
for (String key : uriInfo.getQueryParameters().keySet()) {
@@ -815,7 +811,7 @@ private Response downloadDatafiles(User user, String rawFileIds, boolean donotwr
815811
if (useCustomZipService) {
816812
URI redirect_uri = null;
817813
try {
818-
redirect_uri = handleCustomZipDownload(user, customZipServiceUrl, fileIds, apiTokenUser, uriInfo, headers, donotwriteGBResponse, true);
814+
redirect_uri = handleCustomZipDownload(user, customZipServiceUrl, fileIds, uriInfo, headers, donotwriteGBResponse, true);
819815
} catch (WebApplicationException wae) {
820816
throw wae;
821817
}
@@ -860,7 +856,7 @@ public void write(OutputStream os) throws IOException,
860856
logger.fine("adding datafile (id=" + file.getId() + ") to the download list of the ZippedDownloadInstance.");
861857
//downloadInstance.addDataFile(file);
862858
if (donotwriteGBResponse != true && file.isReleased()){
863-
GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, apiTokenUser);
859+
GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, user);
864860
guestbookResponseService.save(gbr);
865861
MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, file);
866862
mdcLogService.logEntry(entry);
@@ -1735,15 +1731,22 @@ public Response getUserPermissionsOnFile(@Context ContainerRequestContext crc, @
17351731
}
17361732

17371733
// checkAuthorization is a convenience method; it calls the boolean method
1738-
// isAccessAuthorized(), the actual workhorse, tand throws a 403 exception if not.
1739-
1740-
private void checkAuthorization(User user, DataFile df) throws WebApplicationException {
1741-
1734+
// isAccessAuthorized(), the actual workhorse, and throws a 403 exception if not.
1735+
private void checkAuthorization(ContainerRequestContext crc, DataFile df) throws WebApplicationException {
1736+
User user = getUser(crc);
17421737
if (!isAccessAuthorized(user, df)) {
17431738
throw new ForbiddenException();
17441739
}
17451740
}
1746-
1741+
private User getUser(ContainerRequestContext crc) {
1742+
User user = getRequestUser(crc);
1743+
// CompoundAuthMechanism should find the user by API Key/Token, Workflow, etc. And for SPA the Bearer Token
1744+
// For JSF check if CompoundAuthMechanism couldn't find the user then try to get it from the session
1745+
if (session!=null && user instanceof GuestUser) {
1746+
user = session.getUser();
1747+
}
1748+
return user;
1749+
}
17471750

17481751
private boolean isAccessAuthorized(User requestUser, DataFile df) {
17491752
// First, check if the file belongs to a released Dataset version:
@@ -1818,8 +1821,6 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
18181821
}
18191822
}
18201823
}
1821-
1822-
18231824

18241825
//The one case where we don't need to check permissions
18251826
if (!restricted && !embargoed && !retentionExpired && published) {
@@ -1828,64 +1829,20 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
18281829
// be handled below)
18291830
return true;
18301831
}
1831-
1832-
//For permissions check decide if we have a session user, or an API user
1833-
User sessionUser = null;
1834-
1832+
18351833
/**
18361834
* Authentication/authorization:
18371835
*/
18381836

1839-
User apiUser = requestUser;
1840-
1841-
/*
1842-
* If API user is not authenticated, and a session user exists, we use that.
1843-
* If the API user indicates a GuestUser, we will use that if there's no session.
1844-
*
1845-
* This is currently the only API call that supports sessions. If the rest of
1846-
* the API is opened up, the custom logic here wouldn't be needed.
1847-
*/
1848-
1849-
if ((apiUser instanceof GuestUser) && session != null) {
1850-
if (session.getUser() != null) {
1851-
sessionUser = session.getUser();
1852-
apiUser = null;
1853-
//Fine logging
1854-
if (!session.getUser().isAuthenticated()) {
1855-
logger.fine("User associated with the session is not an authenticated user.");
1856-
if (session.getUser() instanceof PrivateUrlUser) {
1857-
logger.fine("User associated with the session is a PrivateUrlUser user.");
1858-
}
1859-
if (session.getUser() instanceof GuestUser) {
1860-
logger.fine("User associated with the session is indeed a guest user.");
1861-
}
1862-
}
1863-
} else {
1864-
logger.fine("No user associated with the session.");
1865-
}
1866-
} else {
1867-
logger.fine("Session is null.");
1868-
}
1869-
//If we don't have a user, nothing more to do. (Note session could have returned GuestUser)
1870-
if (sessionUser == null && apiUser == null) {
1871-
logger.warning("Unable to find a user via session or with a token.");
1872-
return false;
1873-
}
1874-
18751837
/*
18761838
* Since published and not restricted/embargoed is handled above, the main split
18771839
* now is whether it is published or not. If it's published, the only case left
18781840
* is with restricted/embargoed. With unpublished, both the restricted/embargoed
18791841
* and not restricted/embargoed both get handled the same way.
18801842
*/
18811843

1882-
DataverseRequest dvr = null;
1883-
if (apiUser != null) {
1884-
dvr = createDataverseRequest(apiUser);
1885-
} else {
1886-
// used in JSF context, user may be Guest
1887-
dvr = dvRequestService.getDataverseRequest();
1888-
}
1844+
DataverseRequest dvr = createDataverseRequest(requestUser);
1845+
18891846
if (!published) { // and restricted or embargoed (implied by earlier processing)
18901847
// If the file is not published, they can still download the file, if the user
18911848
// has the permission to view unpublished versions:
@@ -1895,7 +1852,7 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
18951852
// it's not unthinkable, that a GuestUser could be given
18961853
// the ViewUnpublished permission!
18971854
logger.log(Level.FINE,
1898-
"Session-based auth: user {0} has access rights on the non-restricted, unpublished datafile.",
1855+
"auth: user {0} has access rights on the non-restricted, unpublished datafile.",
18991856
dvr.getUser().getIdentifier());
19001857
return true;
19011858
}
@@ -1905,35 +1862,11 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
19051862
return true;
19061863
}
19071864
}
1908-
if (sessionUser != null) {
1909-
logger.log(Level.FINE, "Session-based auth: user {0} has NO access rights on the requested datafile.", sessionUser.getIdentifier());
1910-
}
1911-
1912-
if (apiUser != null) {
1913-
logger.log(Level.FINE, "Token-based auth: user {0} has NO access rights on the requested datafile.", apiUser.getIdentifier());
1914-
}
1915-
return false;
1916-
}
1917-
19181865

1919-
1920-
private User findAPITokenUser(User requestUser) {
1921-
User apiTokenUser = requestUser;
1922-
/*
1923-
* The idea here is to not let a guest user coming from the request (which
1924-
* happens when there is no key/token, and which we want if there's no session)
1925-
* from overriding an authenticated session user.
1926-
*/
1927-
if(apiTokenUser instanceof GuestUser) {
1928-
if(session!=null && session.getUser()!=null) {
1929-
//The apiTokenUser, if set, will override the sessionUser in permissions calcs, so set it to null if we have a session user
1930-
apiTokenUser=null;
1931-
}
1932-
}
1933-
return apiTokenUser;
1866+
return false;
19341867
}
19351868

1936-
private URI handleCustomZipDownload(User user, String customZipServiceUrl, String fileIds, User apiTokenUser, UriInfo uriInfo, HttpHeaders headers, boolean donotwriteGBResponse, boolean orig) throws WebApplicationException {
1869+
private URI handleCustomZipDownload(User user, String customZipServiceUrl, String fileIds, UriInfo uriInfo, HttpHeaders headers, boolean donotwriteGBResponse, boolean orig) throws WebApplicationException {
19371870

19381871
String zipServiceKey = null;
19391872
Timestamp timestamp = null;
@@ -1962,7 +1895,7 @@ private URI handleCustomZipDownload(User user, String customZipServiceUrl, Strin
19621895
if (isAccessAuthorized(user, file)) {
19631896
logger.fine("adding datafile (id=" + file.getId() + ") to the download list of the ZippedDownloadInstance.");
19641897
if (donotwriteGBResponse != true && file.isReleased()) {
1965-
GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, apiTokenUser);
1898+
GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, user);
19661899
guestbookResponseService.save(gbr);
19671900
MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, file);
19681901
mdcLogService.logEntry(entry);

0 commit comments

Comments
 (0)