Skip to content

Commit

Permalink
[CA-849] Trace auth calls (#434)
Browse files Browse the repository at this point in the history
* `withChildTraceSpan` is a trace Directive that will create a child span
  • Loading branch information
andy7i committed May 29, 2020
1 parent 8e80892 commit 352225d
Show file tree
Hide file tree
Showing 11 changed files with 170 additions and 71 deletions.
2 changes: 1 addition & 1 deletion automation/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ LOCAL_UI=true ./render-local-env.sh
```

##### Against a local Sam
Run `./render-local-env.sh` and then update the URIs in `src/test/resources/application.conf` to:
Run `./render-local-env.sh` and then update the URIs in `automation/src/test/resources/application.conf` to:
```
baseUrl = "https://firecloud.dsde-dev.broadinstitute.org/"
orchApiUrl = "https://firecloud-orchestration.dsde-dev.broadinstitute.org/"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ import scala.concurrent.ExecutionContext
/**
* Created by gpolumbo on 2/20/2018.
*/
trait ManagedGroupRoutes extends UserInfoDirectives with SecurityDirectives with SamModelDirectives {
trait ManagedGroupRoutes extends UserInfoDirectives with SecurityDirectives with SamModelDirectives with SamRequestContextDirectives {
implicit val executionContext: ExecutionContext

val managedGroupService: ManagedGroupService

def groupRoutes: server.Route = withSamRequestContext { samRequestContext =>
requireUserInfo { userInfo =>
requireUserInfo(samRequestContext) { userInfo =>
(pathPrefix("groups" / "v1") | pathPrefix("group")) {
pathPrefix(Segment) { groupId =>
val managedGroup = FullyQualifiedResourceId(ManagedGroupService.managedGroupTypeName, ResourceId(groupId))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext
/**
* Created by mbemis on 5/22/17.
*/
trait ResourceRoutes extends UserInfoDirectives with SecurityDirectives with SamModelDirectives {
trait ResourceRoutes extends UserInfoDirectives with SecurityDirectives with SamModelDirectives with SamRequestContextDirectives {
implicit val executionContext: ExecutionContext
val resourceService: ResourceService
val liquibaseConfig: LiquibaseConfig
Expand All @@ -43,7 +43,7 @@ trait ResourceRoutes extends UserInfoDirectives with SecurityDirectives with Sam

def resourceRoutes: server.Route =
pathPrefix("initializeResourceTypes") {
requireUserInfo { userInfo =>
requireUserInfo(SamRequestContext(None)) { userInfo => // `SamRequestContext(None)` is used so that we don't trace 1-off boot/init methods ; these in particular are unpublished APIs
asWorkbenchAdmin(userInfo) {
pathEndOrSingleSlash {
put {
Expand All @@ -54,7 +54,7 @@ trait ResourceRoutes extends UserInfoDirectives with SecurityDirectives with Sam
}
} ~
(pathPrefix("config" / "v1" / "resourceTypes") | pathPrefix("resourceTypes")) {
requireUserInfo { userInfo =>
requireUserInfo(SamRequestContext(None)) { userInfo => // `SamRequestContext(None)` is used so that we don't trace 1-off boot/init methods ; these in particular are unpublished APIs
pathEndOrSingleSlash {
get {
complete(resourceService.getResourceTypes().map(typeMap => StatusCodes.OK -> typeMap.values.toSet))
Expand All @@ -64,7 +64,7 @@ trait ResourceRoutes extends UserInfoDirectives with SecurityDirectives with Sam
} ~
(pathPrefix("resources" / "v1") | pathPrefix("resource")) {
withSamRequestContext { samRequestContext =>
requireUserInfo { userInfo =>
requireUserInfo(samRequestContext) { userInfo =>
pathPrefix(Segment) { resourceTypeName =>
withResourceType(ResourceTypeName(resourceTypeName)) { resourceType =>
pathEndOrSingleSlash {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import akka.http.scaladsl.model.StatusCodes
import akka.http.scaladsl.server.Directive1
import akka.http.scaladsl.server.Directives.{onSuccess, _}
import akka.http.scaladsl.unmarshalling.FromRequestUnmarshaller
import io.opencensus.scala.akka.http.TracingDirective.traceRequest
import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.sam._
import org.broadinstitute.dsde.workbench.sam.service.UserService
Expand All @@ -31,8 +30,4 @@ trait SamModelDirectives {
}
}

def withSamRequestContext: Directive1[SamRequestContext] =
traceRequest.map { span => SamRequestContext(Option(span)) }


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package org.broadinstitute.dsde.workbench.sam.api

import akka.http.scaladsl.model.HttpResponse
import akka.http.scaladsl.server.Directives._
import akka.http.scaladsl.server.{Directive1, ExceptionHandler}
import io.opencensus.scala.Tracing
import io.opencensus.scala.Tracing.startSpanWithParent
import io.opencensus.scala.akka.http.TracingDirective.traceRequest
import io.opencensus.scala.akka.http.trace.HttpExtractors._
import io.opencensus.scala.akka.http.utils.ExecuteAfterResponse
import io.opencensus.scala.http.{HttpAttributes, StatusTranslator}
import io.opencensus.trace.{Span, Status}
import org.broadinstitute.dsde.workbench.sam.service.UserService
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext

import scala.util.control.NonFatal

/**
* Created by ajang on 2020-05-28
*/
trait SamRequestContextDirectives {
val userService: UserService

/**
* Provides a new SamRequestContext with a root tracing span.
*/
def withSamRequestContext: Directive1[SamRequestContext] =
traceRequest.map { span => SamRequestContext(Option(span)) }

/**
* Provides a new SamRequestContext with a tracing span that is a child of the existing SamRequestContext's
* `parentSpan`. If a parentSpan does not exist, this will NOT create a new one.
*
* @param spanName name of the new parentSpan in the new SamRequestContext. This span is a child of the existing
* parentSpan.
* @param samRequestContext the existing samRequestContext.
*/
def withChildTraceSpan(spanName: String, samRequestContext: SamRequestContext): Directive1[SamRequestContext] =
samRequestContext.parentSpan match {
case Some (parentSpan) =>
val newSpan = startSpanWithParent(spanName, parentSpan)
val newSamRequestContext = samRequestContext.copy(parentSpan = Option(newSpan))
recordSuccess(newSpan) & recordException(newSpan) & provide(newSamRequestContext)
case None => provide(SamRequestContext(None))
}

// The private methods and objects below are copied wholesale from io/opencensus/scala/akka/http/TracingDirective
// which seems to be the lesser evil compared to accessing the package's private methods.
private def tracing: Tracing = Tracing
private def recordSuccess(span: Span) =
mapResponse(EndSpanResponse.forServer(tracing, _, span))
private def recordException(span: Span) =
handleExceptions(ExceptionHandler {
case NonFatal(ex) =>
tracing.endSpan(span, Status.INTERNAL)
throw ex
})
private object EndSpanResponse {

def forServer(
tracing: Tracing,
response: HttpResponse,
span: Span
): HttpResponse =
end(tracing, response, span, "response sent")

def forClient(
tracing: Tracing,
response: HttpResponse,
span: Span
): HttpResponse =
end(tracing, response, span, "response received")

private def end(
tracing: Tracing,
response: HttpResponse,
span: Span,
responseAnnotation: String
): HttpResponse = {
HttpAttributes.setAttributesForResponse(span, response)
span.addAnnotation(responseAnnotation)
tracing.setStatus(
span,
StatusTranslator.translate(response.status.intValue())
)

ExecuteAfterResponse.onComplete(
response,
onFinish = () => tracing.endSpan(span),
onFailure = _ => tracing.endSpan(span)
)
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ import DefaultJsonProtocol._
import WorkbenchIdentityJsonSupport.identityConcentratorIdFormat
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext

trait StandardUserInfoDirectives extends UserInfoDirectives with LazyLogging {
trait StandardUserInfoDirectives extends UserInfoDirectives with LazyLogging with SamRequestContextDirectives {
implicit val executionContext: ExecutionContext
val identityConcentratorService: Option[IdentityConcentratorService] = None

def requireUserInfo: Directive1[UserInfo] = handleAuthHeaders(requireUserInfoFromJwt, requireUserInfoFromOIDC)
def requireCreateUser: Directive1[CreateWorkbenchUser] = handleAuthHeaders(requireCreateUserInfoFromJwt, requireCreateUserInfoFromOIDC)
def requireUserInfo(samRequestContext: SamRequestContext): Directive1[UserInfo] = handleAuthHeaders(requireUserInfoFromJwt, requireUserInfoFromOIDC, samRequestContext)
def requireCreateUser(samRequestContext: SamRequestContext): Directive1[CreateWorkbenchUser] = handleAuthHeaders(requireCreateUserInfoFromJwt, requireCreateUserInfoFromOIDC, samRequestContext)

/**
* Utility function that knows how to handle the request based on whether or not JWT is present.
Expand All @@ -42,46 +42,52 @@ trait StandardUserInfoDirectives extends UserInfoDirectives with LazyLogging {
* @tparam T type this request returns
* @return
*/
private def handleAuthHeaders[T](fromJwt: (JwtUserInfo, OAuth2BearerToken, IdentityConcentratorService) => Directive1[T], fromOIDC: OIDCHeaders => Directive1[T]): Directive1[T] =
(headerValueByName(authorizationHeader) &
provide(identityConcentratorService)) tflatMap {
// order of these cases is important: if the authorization header is a valid jwt we must ignore
// any OIDC headers otherwise we may be vulnerable to a malicious user specifying a valid JWT
// but different user information in the OIDC headers
case (JwtAuthorizationHeader(Success(jwtUserInfo), bearerToken), Some(icService)) =>
fromJwt(jwtUserInfo, bearerToken, icService)

case (JwtAuthorizationHeader(Failure(regrets), _), _) =>
failWith(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.Unauthorized, "could not parse authorization header, jwt missing required fields", regrets)))

case (JwtAuthorizationHeader(_, _), None) =>
logger.error("requireUserInfo with JWT attempted but Identity Concentrator not configured")
failWith(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.InternalServerError, "Identity Concentrator not configured")))

case _ =>
// request coming through Apache proxy with OIDC headers
(headerValueByName(accessTokenHeader).as(OAuth2BearerToken) &
headerValueByName(googleSubjectIdHeader).as(GoogleSubjectId) &
headerValueByName(expiresInHeader) &
headerValueByName(emailHeader).as(WorkbenchEmail)).as(OIDCHeaders).flatMap(fromOIDC)
private def handleAuthHeaders[T](fromJwt: (JwtUserInfo, OAuth2BearerToken, IdentityConcentratorService, SamRequestContext) => Directive1[T], fromOIDC: (OIDCHeaders, SamRequestContext) => Directive1[T], samRequestContext: SamRequestContext): Directive1[T] =
withChildTraceSpan("handleAuthHeaders", samRequestContext).flatMap { samRequestContext =>
(headerValueByName(authorizationHeader) &
provide(identityConcentratorService)) tflatMap {
// order of these cases is important: if the authorization header is a valid jwt we must ignore
// any OIDC headers otherwise we may be vulnerable to a malicious user specifying a valid JWT
// but different user information in the OIDC headers
case (JwtAuthorizationHeader(Success(jwtUserInfo), bearerToken), Some(icService)) =>
withChildTraceSpan("JWT-request", samRequestContext).flatMap { samRequestContext =>
fromJwt(jwtUserInfo, bearerToken, icService, samRequestContext)
}

case (JwtAuthorizationHeader(Failure(regrets), _), _) =>
failWith(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.Unauthorized, "could not parse authorization header, jwt missing required fields", regrets)))

case (JwtAuthorizationHeader(_, _), None) =>
logger.error("requireUserInfo with JWT attempted but Identity Concentrator not configured")
failWith(new WorkbenchExceptionWithErrorReport(ErrorReport(StatusCodes.InternalServerError, "Identity Concentrator not configured")))

case _ =>
// request coming through Apache proxy with OIDC headers
withChildTraceSpan("OIDC-request", samRequestContext).flatMap { samRequestContext =>
(headerValueByName(accessTokenHeader).as(OAuth2BearerToken) &
headerValueByName(googleSubjectIdHeader).as(GoogleSubjectId) &
headerValueByName(expiresInHeader) &
headerValueByName(emailHeader).as(WorkbenchEmail)).as(OIDCHeaders).flatMap(fromOIDC(_, samRequestContext))
}
}
}

private def requireUserInfoFromJwt(jwtUserInfo: JwtUserInfo, bearerToken: OAuth2BearerToken, icService: IdentityConcentratorService): Directive1[UserInfo] =
private def requireUserInfoFromJwt(jwtUserInfo: JwtUserInfo, bearerToken: OAuth2BearerToken, icService: IdentityConcentratorService, samRequestContext: SamRequestContext): Directive1[UserInfo] =
onSuccess {
getUserInfoFromJwt(jwtUserInfo, bearerToken, directoryDAO, icService, SamRequestContext(None)).unsafeToFuture() // Plumbing for this tracewill be added in a follow-up PR. Ticket: https://broadworkbench.atlassian.net/browse/CA-849
getUserInfoFromJwt(jwtUserInfo, bearerToken, directoryDAO, icService, samRequestContext).unsafeToFuture()
}

private def requireUserInfoFromOIDC(oidcHeaders: OIDCHeaders): Directive1[UserInfo] =
private def requireUserInfoFromOIDC(oidcHeaders: OIDCHeaders, samRequestContext: SamRequestContext): Directive1[UserInfo] =
onSuccess {
getUserInfoFromOidcHeaders(directoryDAO, oidcHeaders, SamRequestContext(None)).unsafeToFuture() // Plumbing for this tracewill be added in a follow-up PR. Ticket: https://broadworkbench.atlassian.net/browse/CA-849
getUserInfoFromOidcHeaders(directoryDAO, oidcHeaders, samRequestContext).unsafeToFuture()
}

private def requireCreateUserInfoFromJwt(jwtUserInfo: JwtUserInfo, bearerToken: OAuth2BearerToken, icService: IdentityConcentratorService): Directive1[CreateWorkbenchUser] =
private def requireCreateUserInfoFromJwt(jwtUserInfo: JwtUserInfo, bearerToken: OAuth2BearerToken, icService: IdentityConcentratorService, samRequestContext: SamRequestContext): Directive1[CreateWorkbenchUser] =
onSuccess {
newCreateWorkbenchUserFromJwt(jwtUserInfo, bearerToken, icService).unsafeToFuture()
}

private def requireCreateUserInfoFromOIDC(oidcHeaders: OIDCHeaders): Directive1[CreateWorkbenchUser] =
private def requireCreateUserInfoFromOIDC(oidcHeaders: OIDCHeaders, samRequestContext: SamRequestContext): Directive1[CreateWorkbenchUser] =
provide(CreateWorkbenchUser(genWorkbenchUserId(System.currentTimeMillis()), oidcHeaders.googleSubjectId, oidcHeaders.email, None))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.broadinstitute.dsde.workbench.model._
import org.broadinstitute.dsde.workbench.sam.service.CloudExtensions
import org.broadinstitute.dsde.workbench.sam._
import org.broadinstitute.dsde.workbench.sam.directory.DirectoryDAO
import org.broadinstitute.dsde.workbench.sam.util.SamRequestContext

/**
* Directives to get user information.
Expand All @@ -15,9 +16,9 @@ trait UserInfoDirectives {
val directoryDAO: DirectoryDAO
val cloudExtensions: CloudExtensions

def requireUserInfo: Directive1[UserInfo]
def requireUserInfo(samRequestContext: SamRequestContext): Directive1[UserInfo]

def requireCreateUser: Directive1[CreateWorkbenchUser]
def requireCreateUser(samRequestContext: SamRequestContext): Directive1[CreateWorkbenchUser]

def asWorkbenchAdmin(userInfo: UserInfo): Directive0 =
Directives.mapInnerRoute { r =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import scala.concurrent.ExecutionContext
/**
* Created by mbemis on 5/22/17.
*/
trait UserRoutes extends UserInfoDirectives with SamModelDirectives {
trait UserRoutes extends UserInfoDirectives with SamRequestContextDirectives {
implicit val executionContext: ExecutionContext
val userService: UserService

Expand All @@ -25,14 +25,14 @@ trait UserRoutes extends UserInfoDirectives with SamModelDirectives {
pathEndOrSingleSlash {
post {
withSamRequestContext { samRequestContext =>
requireCreateUser { createUser =>
requireCreateUser(samRequestContext) { createUser =>
complete {
userService.createUser(createUser, samRequestContext).map(userStatus => StatusCodes.Created -> userStatus)
}
}
}
} ~ withSamRequestContext { samRequestContext =>
requireUserInfo { user =>
requireUserInfo(samRequestContext) { user =>
get {
parameter("userDetailsOnly".?) { userDetailsOnly =>
complete {
Expand All @@ -54,15 +54,15 @@ trait UserRoutes extends UserInfoDirectives with SamModelDirectives {
pathEndOrSingleSlash {
post {
withSamRequestContext { samRequestContext =>
requireCreateUser { createUser =>
requireCreateUser(samRequestContext) { createUser =>
complete {
userService.createUser(createUser, samRequestContext).map(userStatus => StatusCodes.Created -> userStatus)
}
}
}
}
} ~ withSamRequestContext { samRequestContext =>
requireUserInfo { user =>
requireUserInfo(samRequestContext) { user =>
path("info") {
get {
complete {
Expand Down Expand Up @@ -98,7 +98,7 @@ trait UserRoutes extends UserInfoDirectives with SamModelDirectives {
def adminUserRoutes: server.Route =
pathPrefix("admin") {
withSamRequestContext { samRequestContext =>
requireUserInfo { userInfo =>
requireUserInfo(samRequestContext) { userInfo =>
asWorkbenchAdmin(userInfo) {
pathPrefix("user") {
path("email" / Segment) { email =>
Expand Down Expand Up @@ -182,7 +182,7 @@ trait UserRoutes extends UserInfoDirectives with SamModelDirectives {
val apiUserRoutes: server.Route = pathPrefix("users") {
pathPrefix("v1") {
withSamRequestContext { samRequestContext =>
requireUserInfo { userInfo =>
requireUserInfo(samRequestContext) { userInfo =>
get {
path(Segment) { email =>
pathEnd {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import akka.http.scaladsl.server.Directives._
import org.broadinstitute.dsde.workbench.model.WorkbenchIdentityJsonSupport._
import org.broadinstitute.dsde.workbench.model.google._
import org.broadinstitute.dsde.workbench.model.{ErrorReport, WorkbenchEmail, WorkbenchExceptionWithErrorReport, WorkbenchUser}
import org.broadinstitute.dsde.workbench.sam.api.{ExtensionRoutes, SamModelDirectives, SecurityDirectives, UserInfoDirectives, ioMarshaller}
import org.broadinstitute.dsde.workbench.sam.api.{ExtensionRoutes, SamModelDirectives, SamRequestContextDirectives, SecurityDirectives, UserInfoDirectives, ioMarshaller}
import org.broadinstitute.dsde.workbench.sam.model.SamJsonSupport._
import org.broadinstitute.dsde.workbench.sam.model._
import org.broadinstitute.dsde.workbench.sam.service.CloudExtensions
Expand All @@ -17,15 +17,15 @@ import spray.json.JsString

import scala.concurrent.ExecutionContext

trait GoogleExtensionRoutes extends ExtensionRoutes with UserInfoDirectives with SecurityDirectives with SamModelDirectives {
trait GoogleExtensionRoutes extends ExtensionRoutes with UserInfoDirectives with SecurityDirectives with SamModelDirectives with SamRequestContextDirectives {
implicit val executionContext: ExecutionContext
val googleExtensions: GoogleExtensions
val googleGroupSynchronizer: GoogleGroupSynchronizer

override def extensionRoutes: server.Route =
(pathPrefix("google" / "v1") | pathPrefix("google")) {
withSamRequestContext { samRequestContext =>
requireUserInfo { userInfo =>
requireUserInfo(samRequestContext) { userInfo =>
path("petServiceAccount" / Segment / Segment) { (project, userEmail) =>
get {
requireAction(
Expand Down
Loading

0 comments on commit 352225d

Please sign in to comment.