Address tlang's comments

broadinstitute · Nov 6, 2023 · 35776ed · 35776ed
1 parent 70038ad
commit 35776ed
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 25 deletions.
diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/SamRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/SamRoutes.scala
@@ -74,16 +74,14 @@ abstract class SamRoutes(
           userRoutesV2(samRequestContext) ~
           publicTermsOfServiceRoutes ~
           withActiveUser(samRequestContext) { samUser =>
-            isWorkbenchAdmin(samUser) { isAdmin =>
-              val samRequestContextWithUser = samRequestContext.copy(samUser = Option(samUser))
-              resourceRoutes(samUser, samRequestContextWithUser) ~
-              adminRoutes(samUser, samRequestContextWithUser) ~
-              extensionRoutes(samUser, samRequestContextWithUser) ~
-              groupRoutes(samUser, samRequestContextWithUser) ~
-              azureRoutes(samUser, samRequestContextWithUser) ~
-              userRoutesV1(samUser, samRequestContextWithUser) ~
-              userTermsOfServiceRoutes(samUser, isAdmin, samRequestContextWithUser)
-            }
+            val samRequestContextWithUser = samRequestContext.copy(samUser = Option(samUser))
+            resourceRoutes(samUser, samRequestContextWithUser) ~
+            adminRoutes(samUser, samRequestContextWithUser) ~
+            extensionRoutes(samUser, samRequestContextWithUser) ~
+            groupRoutes(samUser, samRequestContextWithUser) ~
+            azureRoutes(samUser, samRequestContextWithUser) ~
+            userRoutesV1(samUser, samRequestContextWithUser) ~
+            userTermsOfServiceRoutes(samUser, samRequestContextWithUser)
           }
         }
       }

diff --git a/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/TermsOfServiceRoutes.scala b/src/main/scala/org/broadinstitute/dsde/workbench/sam/api/TermsOfServiceRoutes.scala
@@ -13,7 +13,7 @@ import org.broadinstitute.dsde.workbench.sam.model.api.SamJsonSupport._
 import scala.concurrent.ExecutionContext
 import scala.util.matching.Regex
 
-trait TermsOfServiceRoutes {
+trait TermsOfServiceRoutes extends SamUserDirectives {
   val tosService: TosService
   implicit val executionContext: ExecutionContext
   private val samUserIdPattern: Regex = "^[a-zA-Z0-9]+$".r
@@ -64,14 +64,16 @@ trait TermsOfServiceRoutes {
       }
     }
 
-  def userTermsOfServiceRoutes(samUser: SamUser, isAdmin: Boolean, samRequestContext: SamRequestContext): server.Route =
+  def userTermsOfServiceRoutes(samUser: SamUser, samRequestContext: SamRequestContext): server.Route =
     pathPrefix("termsOfService") {
       pathPrefix("v1") {
         pathPrefix("user") { // api/termsOfService/v1/user
           pathPrefix("self") { // api/termsOfService/v1/user/self
             pathEndOrSingleSlash {
               get {
-                complete(StatusCodes.OK, tosService.getTermsOfServiceDetailsForUser(samUser.id, samUser, isAdmin = false, samRequestContext))
+                complete {
+                  tosService.getTermsOfServiceDetailsForUser(samUser.id, samUser, isAdmin = false, samRequestContext)
+                }
               }
             } ~
             pathPrefix("accept") { // api/termsOfService/v1/user/accept
@@ -94,8 +96,12 @@ trait TermsOfServiceRoutes {
             validate(samUserIdPattern.matches(userId), "User ID must be alpha numeric") {
               val requestUserId = WorkbenchUserId(userId)
               pathEndOrSingleSlash {
-                get {
-                  complete(StatusCodes.OK, tosService.getTermsOfServiceDetailsForUser(requestUserId, samUser, isAdmin, samRequestContext))
+                isWorkbenchAdmin(samUser) { isAdmin =>
+                  get {
+                    complete {
+                      tosService.getTermsOfServiceDetailsForUser(requestUserId, samUser, isAdmin, samRequestContext)
+                    }
+                  }
                 }
               } ~
               pathPrefix("history") { // api/termsOfService/v1/user/{userId}/history

diff --git a/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/MockSamRoutes.scala b/src/test/scala/org/broadinstitute/dsde/workbench/sam/api/MockSamRoutes.scala
@@ -68,16 +68,14 @@ abstract class MockSamRoutes(
               // IMPORTANT - all routes under /api must have an active user
               publicTermsOfServiceRoutes ~
                 withActiveUser(samRequestContext) { samUser =>
-                  isWorkbenchAdmin(samUser) { isAdmin =>
-                    val samRequestContextWithUser = samRequestContext.copy(samUser = Option(samUser))
-                    resourceRoutes(samUser, samRequestContextWithUser) ~
-                      adminRoutes(samUser, samRequestContextWithUser) ~
-                      extensionRoutes(samUser, samRequestContextWithUser) ~
-                      groupRoutes(samUser, samRequestContextWithUser) ~
-                      userRoutesV1(samUser, samRequestContextWithUser) ~
-                      azureRoutes(samUser, samRequestContextWithUser) ~
-                      userTermsOfServiceRoutes(samUser, isAdmin, samRequestContextWithUser)
-                  }
+                  val samRequestContextWithUser = samRequestContext.copy(samUser = Option(samUser))
+                  resourceRoutes(samUser, samRequestContextWithUser) ~
+                    adminRoutes(samUser, samRequestContextWithUser) ~
+                    extensionRoutes(samUser, samRequestContextWithUser) ~
+                    groupRoutes(samUser, samRequestContextWithUser) ~
+                    userRoutesV1(samUser, samRequestContextWithUser) ~
+                    azureRoutes(samUser, samRequestContextWithUser) ~
+                    userTermsOfServiceRoutes(samUser, samRequestContextWithUser)
                 }
             }
         }