Annotate PrepareAction with CSRF not required (#38)

labkey-adam · web-flow · commit de37c8f28870 · 2023-02-14T07:48:45.000-08:00
diff --git a/src/org/labkey/filetransfer/FileTransferController.java b/src/org/labkey/filetransfer/FileTransferController.java
@@ -33,6 +33,7 @@
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerManager;
 import org.labkey.api.security.AdminConsoleAction;
+import org.labkey.api.security.CSRF;
 import org.labkey.api.security.RequiresNoPermission;
 import org.labkey.api.security.RequiresPermission;
 import org.labkey.api.security.User;
@@ -87,7 +88,7 @@ public static ActionURL getComplianceSettingsURL()
     }
 
     @AdminConsoleAction(AdminOperationsPermission.class)
-    public class ConfigurationAction extends FormViewAction<FileTransferConfigForm>
+    public static class ConfigurationAction extends FormViewAction<FileTransferConfigForm>
     {
         @Override
         public void addNavTrail(NavTree root)
@@ -160,11 +161,11 @@ public URLHelper getSuccessURL(FileTransferConfigForm form)
 
     /**
      * This action stores certain properties in the session and then redirects to the authentication provider's
-     * authorization UI.  This redirect contains a parameter indicating the action to return to when authentication is
+     * authorization UI. This redirect contains a parameter indicating the action to return to when authentication is
      * complete.
      */
     @RequiresPermission(ReadPermission.class)
-    public class AuthAction extends SimpleViewAction<TransferSelectionForm>
+    public static class AuthAction extends SimpleViewAction<TransferSelectionForm>
     {
         @Override
         public void addNavTrail(NavTree root)
@@ -203,6 +204,7 @@ public String getDataRegionSelectionKey()
             return dataRegionSelectionKey;
         }
 
+        @SuppressWarnings("unused")
         public void setDataRegionSelectionKey(String dataRegionSelectionKey)
         {
             this.dataRegionSelectionKey = dataRegionSelectionKey;
@@ -213,21 +215,22 @@ public Integer getWebPartId()
             return webPartId;
         }
 
+        @SuppressWarnings("unused")
         public void setWebPartId(Integer webPartId)
         {
             this.webPartId = webPartId;
         }
     }
 
     /**
-     * This action is the target of the authorization action from the file transfer provider.  If authorization has
+     * This action is the target of the authorization action from the file transfer provider. If authorization has
      * been granted, the code provided from that authorization will be used to retrieve credentials to be used in
-     * initiating transfer requests.  In any case, this action redirects to a page where we give feedback to the user
+     * initiating transfer requests. In any case, this action redirects to a page where we give feedback to the user
      * and display items for next steps, which are either to return to the page where the initial selection of files was
      * made, choose a destination endpoint, or initiate a transfer of the selected files.
      */
     @RequiresNoPermission
-    public class TokensAction extends SimpleRedirectAction<AuthForm>
+    public static class TokensAction extends SimpleRedirectAction<AuthForm>
     {
         private boolean authorized = false;
         private ErrorCode errorCode = null;
@@ -306,6 +309,7 @@ public String getCode()
             return _code;
         }
 
+        @SuppressWarnings("unused")
         public void setCode(String code)
         {
             _code = code;
@@ -316,21 +320,22 @@ public String getError()
             return _error;
         }
 
+        @SuppressWarnings("unused")
         public void setError(String error)
         {
             _error = error;
         }
     }
 
     @RequiresPermission(ReadPermission.class)
-    public class TransferAction extends MutatingApiAction<TransferRequestForm>
+    public static class TransferAction extends MutatingApiAction<TransferRequestForm>
     {
         @Override
         public Object execute(TransferRequestForm form, BindException errors)
         {
             FileTransferProvider provider = FileTransferManager.get().getProvider(getViewContext());
             if (provider == null)
-                return new SimpleResponse(false, "Count not find File Transfer Provider in this session.");
+                return new SimpleResponse(false, "Could not find File Transfer Provider in this session.");
             TransferEndpoint source = new TransferEndpoint(form.getSourceEndpoint(), form.getSourcePath());
             TransferEndpoint destination = new TransferEndpoint(form.getDestinationEndpoint(), form.getDestinationPath());
             try
@@ -358,6 +363,7 @@ public String getSourceEndpoint()
             return sourceEndpoint;
         }
 
+        @SuppressWarnings("unused")
         public void setSourceEndpoint(String sourceEndpoint)
         {
             this.sourceEndpoint = sourceEndpoint;
@@ -368,6 +374,7 @@ public String getSourcePath()
             return sourcePath;
         }
 
+        @SuppressWarnings("unused")
         public void setSourcePath(String sourcePath)
         {
             this.sourcePath = sourcePath;
@@ -378,6 +385,7 @@ public String getDestinationEndpoint()
             return destinationEndpoint;
         }
 
+        @SuppressWarnings("unused")
         public void setDestinationEndpoint(String destinationEndpoint)
         {
             this.destinationEndpoint = destinationEndpoint;
@@ -388,6 +396,7 @@ public String getDestinationPath()
             return destinationPath;
         }
 
+        @SuppressWarnings("unused")
         public void setDestinationPath(String destinationPath)
         {
             this.destinationPath = destinationPath;
@@ -398,14 +407,16 @@ public String getLabel()
             return label;
         }
 
+        @SuppressWarnings("unused")
         public void setLabel(String label)
         {
             this.label = label;
         }
     }
 
     @RequiresPermission(ReadPermission.class)
-    public class PrepareAction extends SimpleViewAction<PrepareTransferForm>
+    @CSRF(CSRF.Method.NONE) // Globus POSTs to this action, but it's non-mutating, so no need for CSRF
+    public static class PrepareAction extends SimpleViewAction<PrepareTransferForm>
     {
         @Override
         public void addNavTrail(NavTree root)
@@ -455,6 +466,7 @@ public Boolean getAuthorized()
             return authorized;
         }
 
+        @SuppressWarnings("unused")
         public void setAuthorized(Boolean authorized)
         {
             this.authorized = authorized;
@@ -466,16 +478,19 @@ public String getDestinationId()
             return endpoint_id;
         }
 
+        @SuppressWarnings("unused")
         public void setDestinationId(String destinationId)
         {
             endpoint_id = destinationId;
         }
 
+        @SuppressWarnings("unused")
         public String getEndpoint_id()
         {
             return endpoint_id;
         }
 
+        @SuppressWarnings("unused")
         public void setEndpoint_id(String endpoint_id)
         {
             this.endpoint_id = endpoint_id;
@@ -486,6 +501,7 @@ public String getPath()
             return path;
         }
 
+        @SuppressWarnings("unused")
         public void setPath(String path)
         {
             this.path = path;
@@ -496,6 +512,7 @@ public String getLabel()
             return label;
         }
 
+        @SuppressWarnings("unused")
         public void setLabel(String label)
         {
             this.label = label;
@@ -506,6 +523,7 @@ public ErrorCode getErrorCode()
             return errorCode;
         }
 
+        @SuppressWarnings("unused")
         public void setErrorCode(ErrorCode errorCode)
         {
             this.errorCode = errorCode;
@@ -522,19 +540,17 @@ public void testActionPermissions()
             User user = TestContext.get().getUser();
             assertTrue(user.hasSiteAdminPermission());
 
-            FileTransferController controller = new FileTransferController();
-
             // @RequiresPermission(ReadPermission.class)
             assertForReadPermission(user,
-                controller.new PrepareAction(),
-                controller.new TransferAction(),
-                controller.new AuthAction()
+                new PrepareAction(),
+                new TransferAction(),
+                new AuthAction()
             );
 
             // @AdminConsoleAction
             // @RequiresPermission(AdminOperationsPermission.class)
             assertForAdminPermission(ContainerManager.getRoot(), user,
-                    controller.new ConfigurationAction()
+                new ConfigurationAction()
             );
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,7 @@`
`33`	`33`	`import org.labkey.api.data.Container;`
`34`	`34`	`import org.labkey.api.data.ContainerManager;`
`35`	`35`	`import org.labkey.api.security.AdminConsoleAction;`
	`36`	`+import org.labkey.api.security.CSRF;`
`36`	`37`	`import org.labkey.api.security.RequiresNoPermission;`
`37`	`38`	`import org.labkey.api.security.RequiresPermission;`
`38`	`39`	`import org.labkey.api.security.User;`
`@@ -87,7 +88,7 @@ public static ActionURL getComplianceSettingsURL()`
`87`	`88`	`}`
`88`	`89`
`89`	`90`	`@AdminConsoleAction(AdminOperationsPermission.class)`
`90`		`- public class ConfigurationAction extends FormViewAction<FileTransferConfigForm>`
	`91`	`+ public static class ConfigurationAction extends FormViewAction<FileTransferConfigForm>`
`91`	`92`	`{`
`92`	`93`	`@Override`
`93`	`94`	`public void addNavTrail(NavTree root)`
`@@ -160,11 +161,11 @@ public URLHelper getSuccessURL(FileTransferConfigForm form)`
`160`	`161`
`161`	`162`	`/**`
`162`	`163`	`* This action stores certain properties in the session and then redirects to the authentication provider's`
`163`		`- * authorization UI. This redirect contains a parameter indicating the action to return to when authentication is`
	`164`	`+ * authorization UI. This redirect contains a parameter indicating the action to return to when authentication is`
`164`	`165`	`* complete.`
`165`	`166`	`*/`
`166`	`167`	`@RequiresPermission(ReadPermission.class)`
`167`		`- public class AuthAction extends SimpleViewAction<TransferSelectionForm>`
	`168`	`+ public static class AuthAction extends SimpleViewAction<TransferSelectionForm>`
`168`	`169`	`{`
`169`	`170`	`@Override`
`170`	`171`	`public void addNavTrail(NavTree root)`
`@@ -203,6 +204,7 @@ public String getDataRegionSelectionKey()`
`203`	`204`	`return dataRegionSelectionKey;`
`204`	`205`	`}`
`205`	`206`
	`207`	`+ @SuppressWarnings("unused")`
`206`	`208`	`public void setDataRegionSelectionKey(String dataRegionSelectionKey)`
`207`	`209`	`{`
`208`	`210`	`this.dataRegionSelectionKey = dataRegionSelectionKey;`
`@@ -213,21 +215,22 @@ public Integer getWebPartId()`
`213`	`215`	`return webPartId;`
`214`	`216`	`}`
`215`	`217`
	`218`	`+ @SuppressWarnings("unused")`
`216`	`219`	`public void setWebPartId(Integer webPartId)`
`217`	`220`	`{`
`218`	`221`	`this.webPartId = webPartId;`
`219`	`222`	`}`
`220`	`223`	`}`
`221`	`224`
`222`	`225`	`/**`
`223`		`- * This action is the target of the authorization action from the file transfer provider. If authorization has`
	`226`	`+ * This action is the target of the authorization action from the file transfer provider. If authorization has`
`224`	`227`	`* been granted, the code provided from that authorization will be used to retrieve credentials to be used in`
`225`		`- * initiating transfer requests. In any case, this action redirects to a page where we give feedback to the user`
	`228`	`+ * initiating transfer requests. In any case, this action redirects to a page where we give feedback to the user`
`226`	`229`	`* and display items for next steps, which are either to return to the page where the initial selection of files was`
`227`	`230`	`* made, choose a destination endpoint, or initiate a transfer of the selected files.`
`228`	`231`	`*/`
`229`	`232`	`@RequiresNoPermission`
`230`		`- public class TokensAction extends SimpleRedirectAction<AuthForm>`
	`233`	`+ public static class TokensAction extends SimpleRedirectAction<AuthForm>`
`231`	`234`	`{`
`232`	`235`	`private boolean authorized = false;`
`233`	`236`	`private ErrorCode errorCode = null;`
`@@ -306,6 +309,7 @@ public String getCode()`
`306`	`309`	`return _code;`
`307`	`310`	`}`
`308`	`311`
	`312`	`+ @SuppressWarnings("unused")`
`309`	`313`	`public void setCode(String code)`
`310`	`314`	`{`
`311`	`315`	`_code = code;`
`@@ -316,21 +320,22 @@ public String getError()`
`316`	`320`	`return _error;`
`317`	`321`	`}`
`318`	`322`
	`323`	`+ @SuppressWarnings("unused")`
`319`	`324`	`public void setError(String error)`
`320`	`325`	`{`
`321`	`326`	`_error = error;`
`322`	`327`	`}`
`323`	`328`	`}`
`324`	`329`
`325`	`330`	`@RequiresPermission(ReadPermission.class)`
`326`		`- public class TransferAction extends MutatingApiAction<TransferRequestForm>`
	`331`	`+ public static class TransferAction extends MutatingApiAction<TransferRequestForm>`
`327`	`332`	`{`
`328`	`333`	`@Override`
`329`	`334`	`public Object execute(TransferRequestForm form, BindException errors)`
`330`	`335`	`{`
`331`	`336`	`FileTransferProvider provider = FileTransferManager.get().getProvider(getViewContext());`
`332`	`337`	`if (provider == null)`
`333`		`- return new SimpleResponse(false, "Count not find File Transfer Provider in this session.");`
	`338`	`+ return new SimpleResponse(false, "Could not find File Transfer Provider in this session.");`
`334`	`339`	`TransferEndpoint source = new TransferEndpoint(form.getSourceEndpoint(), form.getSourcePath());`
`335`	`340`	`TransferEndpoint destination = new TransferEndpoint(form.getDestinationEndpoint(), form.getDestinationPath());`
`336`	`341`	`try`
`@@ -358,6 +363,7 @@ public String getSourceEndpoint()`
`358`	`363`	`return sourceEndpoint;`
`359`	`364`	`}`
`360`	`365`
	`366`	`+ @SuppressWarnings("unused")`
`361`	`367`	`public void setSourceEndpoint(String sourceEndpoint)`
`362`	`368`	`{`
`363`	`369`	`this.sourceEndpoint = sourceEndpoint;`
`@@ -368,6 +374,7 @@ public String getSourcePath()`
`368`	`374`	`return sourcePath;`
`369`	`375`	`}`
`370`	`376`
	`377`	`+ @SuppressWarnings("unused")`
`371`	`378`	`public void setSourcePath(String sourcePath)`
`372`	`379`	`{`
`373`	`380`	`this.sourcePath = sourcePath;`
`@@ -378,6 +385,7 @@ public String getDestinationEndpoint()`
`378`	`385`	`return destinationEndpoint;`
`379`	`386`	`}`
`380`	`387`
	`388`	`+ @SuppressWarnings("unused")`
`381`	`389`	`public void setDestinationEndpoint(String destinationEndpoint)`
`382`	`390`	`{`
`383`	`391`	`this.destinationEndpoint = destinationEndpoint;`
`@@ -388,6 +396,7 @@ public String getDestinationPath()`
`388`	`396`	`return destinationPath;`
`389`	`397`	`}`
`390`	`398`
	`399`	`+ @SuppressWarnings("unused")`
`391`	`400`	`public void setDestinationPath(String destinationPath)`
`392`	`401`	`{`
`393`	`402`	`this.destinationPath = destinationPath;`
`@@ -398,14 +407,16 @@ public String getLabel()`
`398`	`407`	`return label;`
`399`	`408`	`}`
`400`	`409`
	`410`	`+ @SuppressWarnings("unused")`
`401`	`411`	`public void setLabel(String label)`
`402`	`412`	`{`
`403`	`413`	`this.label = label;`
`404`	`414`	`}`
`405`	`415`	`}`
`406`	`416`
`407`	`417`	`@RequiresPermission(ReadPermission.class)`
`408`		`- public class PrepareAction extends SimpleViewAction<PrepareTransferForm>`
	`418`	`+ @CSRF(CSRF.Method.NONE) // Globus POSTs to this action, but it's non-mutating, so no need for CSRF`
	`419`	`+ public static class PrepareAction extends SimpleViewAction<PrepareTransferForm>`
`409`	`420`	`{`
`410`	`421`	`@Override`
`411`	`422`	`public void addNavTrail(NavTree root)`
`@@ -455,6 +466,7 @@ public Boolean getAuthorized()`
`455`	`466`	`return authorized;`
`456`	`467`	`}`
`457`	`468`
	`469`	`+ @SuppressWarnings("unused")`
`458`	`470`	`public void setAuthorized(Boolean authorized)`
`459`	`471`	`{`
`460`	`472`	`this.authorized = authorized;`
`@@ -466,16 +478,19 @@ public String getDestinationId()`
`466`	`478`	`return endpoint_id;`
`467`	`479`	`}`
`468`	`480`
	`481`	`+ @SuppressWarnings("unused")`
`469`	`482`	`public void setDestinationId(String destinationId)`
`470`	`483`	`{`
`471`	`484`	`endpoint_id = destinationId;`
`472`	`485`	`}`
`473`	`486`
	`487`	`+ @SuppressWarnings("unused")`
`474`	`488`	`public String getEndpoint_id()`
`475`	`489`	`{`
`476`	`490`	`return endpoint_id;`
`477`	`491`	`}`
`478`	`492`
	`493`	`+ @SuppressWarnings("unused")`
`479`	`494`	`public void setEndpoint_id(String endpoint_id)`
`480`	`495`	`{`
`481`	`496`	`this.endpoint_id = endpoint_id;`
`@@ -486,6 +501,7 @@ public String getPath()`
`486`	`501`	`return path;`
`487`	`502`	`}`
`488`	`503`
	`504`	`+ @SuppressWarnings("unused")`
`489`	`505`	`public void setPath(String path)`
`490`	`506`	`{`
`491`	`507`	`this.path = path;`
`@@ -496,6 +512,7 @@ public String getLabel()`
`496`	`512`	`return label;`
`497`	`513`	`}`
`498`	`514`
	`515`	`+ @SuppressWarnings("unused")`
`499`	`516`	`public void setLabel(String label)`
`500`	`517`	`{`
`501`	`518`	`this.label = label;`
`@@ -506,6 +523,7 @@ public ErrorCode getErrorCode()`
`506`	`523`	`return errorCode;`
`507`	`524`	`}`
`508`	`525`
	`526`	`+ @SuppressWarnings("unused")`
`509`	`527`	`public void setErrorCode(ErrorCode errorCode)`
`510`	`528`	`{`
`511`	`529`	`this.errorCode = errorCode;`
`@@ -522,19 +540,17 @@ public void testActionPermissions()`
`522`	`540`	`User user = TestContext.get().getUser();`
`523`	`541`	`assertTrue(user.hasSiteAdminPermission());`
`524`	`542`
`525`		`- FileTransferController controller = new FileTransferController();`
`526`		`-`
`527`	`543`	`// @RequiresPermission(ReadPermission.class)`
`528`	`544`	`assertForReadPermission(user,`
`529`		`- controller.new PrepareAction(),`
`530`		`- controller.new TransferAction(),`
`531`		`- controller.new AuthAction()`
	`545`	`+ new PrepareAction(),`
	`546`	`+ new TransferAction(),`
	`547`	`+ new AuthAction()`
`532`	`548`	`);`
`533`	`549`
`534`	`550`	`// @AdminConsoleAction`
`535`	`551`	`// @RequiresPermission(AdminOperationsPermission.class)`
`536`	`552`	`assertForAdminPermission(ContainerManager.getRoot(), user,`
`537`		`- controller.new ConfigurationAction()`
	`553`	`+ new ConfigurationAction()`
`538`	`554`	`);`
`539`	`555`	`}`
`540`	`556`	`}`