Merge pull request #11844 from IQSS/11740-api-file-download-with-bearer-token

clean up use of session in Access APIs for SPA

Author: landreev

doc/release-notes/11740-api-file-download-with-bearer-token.md

@@ -0,0 +1,7 @@
+## Bug / Not Bug in Dataverse. Bug is in SPA Frontend
+
+Cleaned up Access APIs to localize getting user from session for JSF backward compatibility
+
+This bug requires a front end fix to send the Bearer Token in the API call.
+
+See: #11740

src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java

@@ -770,49 +770,17 @@ private void initCustomQuestions(GuestbookResponse guestbookResponse, Dataset da
         }
     }
 
-    private void setUserDefaultResponses(GuestbookResponse guestbookResponse, DataverseSession session, User userIn) {
-        User user;
-        User sessionUser = session.getUser();
-        
-        if (userIn != null){
-            user = userIn;
-        } else{
-            user = sessionUser;
-        }
-         
-        if (user != null) {
-            guestbookResponse.setEmail(getUserEMail(user));
-            guestbookResponse.setName(getUserName(user));
-            guestbookResponse.setInstitution(getUserInstitution(user));
-            guestbookResponse.setPosition(getUserPosition(user));
-            guestbookResponse.setAuthenticatedUser(getAuthenticatedUser(user));
-        } else {
-            guestbookResponse.setEmail("");
-            guestbookResponse.setName("");
-            guestbookResponse.setInstitution("");
-            guestbookResponse.setPosition("");
-            guestbookResponse.setAuthenticatedUser(null);
-        }
-        guestbookResponse.setSessionId(session.toString());
+    private void setUserDefaultResponses(GuestbookResponse guestbookResponse, DataverseSession session, User user) {
+        guestbookResponse.setEmail(getUserEMail(user));
+        guestbookResponse.setName(getUserName(user));
+        guestbookResponse.setInstitution(getUserInstitution(user));
+        guestbookResponse.setPosition(getUserPosition(user));
+        guestbookResponse.setAuthenticatedUser(getAuthenticatedUser(user));
+        guestbookResponse.setSessionId(session != null ? session.toString() : "");
     }
 
     private void setUserDefaultResponses(GuestbookResponse guestbookResponse, DataverseSession session) {
-        User user = session.getUser();
-        
-        if (user != null) {
-            guestbookResponse.setEmail(getUserEMail(user));
-            guestbookResponse.setName(getUserName(user));
-            guestbookResponse.setInstitution(getUserInstitution(user));
-            guestbookResponse.setPosition(getUserPosition(user));
-            guestbookResponse.setAuthenticatedUser(getAuthenticatedUser(user));
-        } else {
-            guestbookResponse.setEmail("");
-            guestbookResponse.setName("");
-            guestbookResponse.setInstitution("");
-            guestbookResponse.setPosition("");
-            guestbookResponse.setAuthenticatedUser(null);
-        }
-        guestbookResponse.setSessionId(session.toString());
+        setUserDefaultResponses(guestbookResponse, session, session.getUser());
     }
 
     public GuestbookResponse initDefaultGuestbookResponse(Dataset dataset, DataFile dataFile, DataverseSession session) {

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

@@ -15,7 +15,6 @@
 import edu.harvard.iq.dataverse.authorization.Permission;
 import edu.harvard.iq.dataverse.authorization.RoleAssignee;
 import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser;
-import edu.harvard.iq.dataverse.authorization.users.PrivateUrlUser;
 import edu.harvard.iq.dataverse.authorization.users.GuestUser;
 import edu.harvard.iq.dataverse.authorization.users.User;
 import edu.harvard.iq.dataverse.dataaccess.DataAccess;
@@ -185,14 +184,12 @@ public BundleDownloadInstance datafileBundle(@Context ContainerRequestContext cr
 
         DataFile df = findDataFileOrDieWrapper(fileId);
 
-        // This will throw a ForbiddenException if access isn't authorized: 
-        checkAuthorization(getRequestUser(crc), df);
+        // This will throw a ForbiddenException if access isn't authorized:
+        checkAuthorization(crc, df);
 
         if (gbrecs != true && df.isReleased()){
             // Write Guestbook record if not done previously and file is released
-            //This calls findUserOrDie which will retrieve the key param or api token header, or the workflow token header.
-            User apiTokenUser = findAPITokenUser(getRequestUser(crc));
-            gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, apiTokenUser);
+            gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, getRequestor(crc));
             guestbookResponseService.save(gbr);
             MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, df);
             mdcLogService.logEntry(entry);
@@ -283,13 +280,12 @@ public Response datafile(@Context ContainerRequestContext crc, @PathParam("fileI
             // (nobody should ever be using this API on a harvested DataFile)!
         }
 
-        // This will throw a ForbiddenException if access isn't authorized: 
-        checkAuthorization(getRequestUser(crc), df);
+        // This will throw a ForbiddenException if access isn't authorized:
+        checkAuthorization(crc, df);
 
         if (gbrecs != true && df.isReleased()){
             // Write Guestbook record if not done previously and file is released
-            User apiTokenUser = findAPITokenUser(getRequestUser(crc));
-            gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, apiTokenUser);
+            gbr = guestbookResponseService.initAPIGuestbookResponse(df.getOwner(), df, session, getRequestor(crc));
         }
 
         DownloadInfo dInfo = new DownloadInfo(df);
@@ -624,7 +620,7 @@ public DownloadInstance downloadAuxiliaryFile(@Context ContainerRequestContext c
         // as defined for the DataFile itself), and will throw a ForbiddenException 
         // if access is denied:
         if (!publiclyAvailable) {
-            checkAuthorization(getRequestUser(crc), df);
+            checkAuthorization(crc, df);
         }
 
         return downloadInstance;
@@ -644,7 +640,7 @@ public DownloadInstance downloadAuxiliaryFile(@Context ContainerRequestContext c
     public Response postDownloadDatafiles(@Context ContainerRequestContext crc, String fileIds, @QueryParam("gbrecs") boolean gbrecs, @Context UriInfo uriInfo, @Context HttpHeaders headers, @Context HttpServletResponse response) throws WebApplicationException {
 
 
-        return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, null);
+        return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, null);
     }
 
     @GET
@@ -665,7 +661,7 @@ public Response downloadAllFromLatest(@Context ContainerRequestContext crc, @Pat
                     // We don't want downloads from Draft versions to be counted, 
                     // so we are setting the gbrecs (aka "do not write guestbook response") 
                     // variable accordingly:
-                    return downloadDatafiles(getRequestUser(crc), fileIds, true, uriInfo, headers, response, "draft");
+                    return downloadDatafiles(crc, fileIds, true, uriInfo, headers, response, "draft");
                 }
             }
 
@@ -686,7 +682,7 @@ public Response downloadAllFromLatest(@Context ContainerRequestContext crc, @Pat
             }
 
             String fileIds = getFileIdsAsCommaSeparated(latest.getFileMetadatas());
-            return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, latest.getFriendlyVersionNumber());
+            return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, latest.getFriendlyVersionNumber());
         } catch (WrappedResponse wr) {
             return wr.getResponse();
         }
@@ -736,7 +732,7 @@ public Command<DatasetVersion> handleLatestPublished() {
             if (dsv.isDraft()) {
                 gbrecs = true;
             }
-            return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, dsv.getFriendlyVersionNumber().toLowerCase());
+            return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, dsv.getFriendlyVersionNumber().toLowerCase());
         } catch (WrappedResponse wr) {
             return wr.getResponse();
         }
@@ -777,10 +773,10 @@ private String generateMultiFileBundleName(Dataset dataset, String versionTag) {
     @Path("datafiles/{fileIds}")
     @Produces({"application/zip"})
     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 {
-        return downloadDatafiles(getRequestUser(crc), fileIds, gbrecs, uriInfo, headers, response, null);
+        return downloadDatafiles(crc, fileIds, gbrecs, uriInfo, headers, response, null);
     }
 
-    private Response downloadDatafiles(User user, String rawFileIds, boolean donotwriteGBResponse, UriInfo uriInfo, HttpHeaders headers, HttpServletResponse response, String versionTag) throws WebApplicationException /* throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {
+    private Response downloadDatafiles(ContainerRequestContext crc, String rawFileIds, boolean donotwriteGBResponse, UriInfo uriInfo, HttpHeaders headers, HttpServletResponse response, String versionTag) throws WebApplicationException /* throws NotFoundException, ServiceUnavailableException, PermissionDeniedException, AuthorizationRequiredException*/ {
         final long zipDownloadSizeLimit = systemConfig.getZipDownloadLimit();
 
         logger.fine("setting zip download size limit to " + zipDownloadSizeLimit + " bytes.");
@@ -801,8 +797,8 @@ private Response downloadDatafiles(User user, String rawFileIds, boolean donotwr
 
         String customZipServiceUrl = settingsService.getValueForKey(SettingsServiceBean.Key.CustomZipDownloadServiceUrl);
         boolean useCustomZipService = customZipServiceUrl != null; 
-        
-        User apiTokenUser = findAPITokenUser(user); //for use in adding gb records if necessary
+
+        User user = getRequestor(crc);
 
         Boolean getOrig = false;
         for (String key : uriInfo.getQueryParameters().keySet()) {
@@ -815,7 +811,7 @@ private Response downloadDatafiles(User user, String rawFileIds, boolean donotwr
         if (useCustomZipService) {
             URI redirect_uri = null; 
             try {
-                redirect_uri = handleCustomZipDownload(user, customZipServiceUrl, fileIds, apiTokenUser, uriInfo, headers, donotwriteGBResponse, true);
+                redirect_uri = handleCustomZipDownload(user, customZipServiceUrl, fileIds, uriInfo, headers, donotwriteGBResponse, true);
             } catch (WebApplicationException wae) {
                 throw wae;
             }
@@ -860,7 +856,7 @@ public void write(OutputStream os) throws IOException,
                                     logger.fine("adding datafile (id=" + file.getId() + ") to the download list of the ZippedDownloadInstance.");
                                     //downloadInstance.addDataFile(file);
                                     if (donotwriteGBResponse != true && file.isReleased()){
-                                        GuestbookResponse  gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, apiTokenUser);
+                                        GuestbookResponse  gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, user);
                                         guestbookResponseService.save(gbr);
                                         MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, file);                                        
                                         mdcLogService.logEntry(entry);
@@ -1735,15 +1731,22 @@ public Response getUserPermissionsOnFile(@Context ContainerRequestContext crc, @
     }
 
     // checkAuthorization is a convenience method; it calls the boolean method
-    // isAccessAuthorized(), the actual workhorse, tand throws a 403 exception if not.
-    
-    private void checkAuthorization(User user, DataFile df) throws WebApplicationException {
-
+    // isAccessAuthorized(), the actual workhorse, and throws a 403 exception if not.
+    private void checkAuthorization(ContainerRequestContext crc, DataFile df) throws WebApplicationException {
+        User user = getRequestor(crc);
         if (!isAccessAuthorized(user, df)) {
             throw new ForbiddenException();
         }        
     }
-    
+    private User getRequestor(ContainerRequestContext crc) {
+        User user = getRequestUser(crc);
+        // CompoundAuthMechanism should find the user by API Key/Token, Workflow, etc. And for SPA the Bearer Token
+        // For JSF check if CompoundAuthMechanism couldn't find the user then try to get it from the session
+        if (session!=null && user instanceof GuestUser) {
+            user = session.getUser();
+        }
+        return user;
+    }
 
     private boolean isAccessAuthorized(User requestUser, DataFile df) {
     // First, check if the file belongs to a released Dataset version: 
@@ -1818,8 +1821,6 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
                 }
             }
         }
-        
-        
 
         //The one case where we don't need to check permissions
         if (!restricted && !embargoed && !retentionExpired && published) {
@@ -1828,64 +1829,20 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
             // be handled below)
             return true;
         }
-        
-        //For permissions check decide if we have a session user, or an API user
-        User sessionUser = null;
-        
+
         /** 
          * Authentication/authorization:
          */
 
-        User apiUser = requestUser;
-
-        /*
-         * If API user is not authenticated, and a session user exists, we use that.
-         * If the API user indicates a GuestUser, we will use that if there's no session.
-         * 
-         * This is currently the only API call that supports sessions. If the rest of
-         * the API is opened up, the custom logic here wouldn't be needed.
-         */
-
-        if ((apiUser instanceof GuestUser) && session != null) {
-            if (session.getUser() != null) {
-                sessionUser = session.getUser();
-                apiUser = null;
-                //Fine logging
-                if (!session.getUser().isAuthenticated()) {
-                    logger.fine("User associated with the session is not an authenticated user.");
-                    if (session.getUser() instanceof PrivateUrlUser) {
-                        logger.fine("User associated with the session is a PrivateUrlUser user.");
-                    }
-                    if (session.getUser() instanceof GuestUser) {
-                        logger.fine("User associated with the session is indeed a guest user.");
-                    }
-                }
-            } else {
-                logger.fine("No user associated with the session.");
-            }
-        } else {
-            logger.fine("Session is null.");
-        } 
-        //If we don't have a user, nothing more to do. (Note session could have returned GuestUser)
-        if (sessionUser == null && apiUser == null) {
-            logger.warning("Unable to find a user via session or with a token.");
-            return false;
-        }
-
         /*
          * Since published and not restricted/embargoed is handled above, the main split
          * now is whether it is published or not. If it's published, the only case left
          * is with restricted/embargoed. With unpublished, both the restricted/embargoed
          * and not restricted/embargoed both get handled the same way.
          */
 
-        DataverseRequest dvr = null;
-        if (apiUser != null) {
-            dvr = createDataverseRequest(apiUser);
-        } else {
-            // used in JSF context, user may be Guest
-            dvr = dvRequestService.getDataverseRequest();
-        }
+        DataverseRequest dvr = createDataverseRequest(requestUser);
+
         if (!published) { // and restricted or embargoed (implied by earlier processing)
             // If the file is not published, they can still download the file, if the user
             // has the permission to view unpublished versions:
@@ -1895,7 +1852,7 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
                 // it's not unthinkable, that a GuestUser could be given
                 // the ViewUnpublished permission!
                 logger.log(Level.FINE,
-                        "Session-based auth: user {0} has access rights on the non-restricted, unpublished datafile.",
+                        "auth: user {0} has access rights on the non-restricted, unpublished datafile.",
                         dvr.getUser().getIdentifier());
                 return true;
             }
@@ -1905,35 +1862,11 @@ private boolean isAccessAuthorized(User requestUser, DataFile df) {
                 return true;
             }
         }
-        if (sessionUser != null) {
-            logger.log(Level.FINE, "Session-based auth: user {0} has NO access rights on the requested datafile.", sessionUser.getIdentifier());
-        } 
-        
-        if (apiUser != null) {
-            logger.log(Level.FINE, "Token-based auth: user {0} has NO access rights on the requested datafile.", apiUser.getIdentifier());
-        } 
-        return false; 
-    }   
-    
 
-        
-    private User findAPITokenUser(User requestUser) {
-        User apiTokenUser = requestUser;
-        /*
-         * The idea here is to not let a guest user coming from the request (which
-         * happens when there is no key/token, and which we want if there's no session)
-         * from overriding an authenticated session user.
-         */
-        if(apiTokenUser instanceof GuestUser) {
-            if(session!=null && session.getUser()!=null) {
-            //The apiTokenUser, if set, will override the sessionUser in permissions calcs, so set it to null if we have a session user
-            apiTokenUser=null;
-            }
-        }
-        return apiTokenUser;
+        return false; 
     }
 
-    private URI handleCustomZipDownload(User user, String customZipServiceUrl, String fileIds, User apiTokenUser, UriInfo uriInfo, HttpHeaders headers, boolean donotwriteGBResponse, boolean orig) throws WebApplicationException {
+    private URI handleCustomZipDownload(User user, String customZipServiceUrl, String fileIds, UriInfo uriInfo, HttpHeaders headers, boolean donotwriteGBResponse, boolean orig) throws WebApplicationException {
 
         String zipServiceKey = null; 
         Timestamp timestamp = null; 
@@ -1962,7 +1895,7 @@ private URI handleCustomZipDownload(User user, String customZipServiceUrl, Strin
                     if (isAccessAuthorized(user, file)) {
                         logger.fine("adding datafile (id=" + file.getId() + ") to the download list of the ZippedDownloadInstance.");
                         if (donotwriteGBResponse != true && file.isReleased()) {
-                            GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, apiTokenUser);
+                            GuestbookResponse gbr = guestbookResponseService.initAPIGuestbookResponse(file.getOwner(), file, session, user);
                             guestbookResponseService.save(gbr);
                             MakeDataCountEntry entry = new MakeDataCountEntry(uriInfo, headers, dvRequestService, file);
                             mdcLogService.logEntry(entry);