mkotsur / aws-lambda-scala

Writing AWS Lambdas in Scala
MIT License
145 stars 21 forks source link

Serverless Cognito support #24

Closed dgiebert closed 3 years ago

dgiebert commented 3 years ago

Trying to enable AWS Cognito results in the following error:

Attempt to decode value on failed cursor: DownField(principalId),DownField(authorizer),DownField(requestContext)

This is done with the following snippet

authorizer:
  name: authorizer
  arn: arn:aws:cognito-idp:us-east-1:123456789:userpool/us-east-1_XXXXXX

From what I can undestand it tries to get a principalId under authorizer but it does not exist and its not an optional: https://github.com/mkotsur/aws-lambda-scala/blob/master/src/main/scala/io/github/mkotsur/aws/proxy/package.scala#L5

The json looks in a test nodejs hello world like this (removed private data):

"requestContext": {
    "authorizer": {
        "claims": {
            "token_use": "id",
            "auth_time": "1609775553",
            "exp": "Mon Jan 04 16:52:33 UTC 2021",
            "iat": "Mon Jan 04 15:52:33 UTC 2021",
        }
    }
mkotsur commented 3 years ago

Hi @dgiebert, thanks for reporting!

I assume you're using Amazon Cognito User Pools (as described here).

I guess it's safe to say that the library doesn't support Cognito User Pools at the moment. That's because, unfortunately, the ProxyRequest class has RequestContextAuthorizer hardcoded. That worked well for IAM permissions but doesn't scale well for other access control types.

I think that it's worthwhile to fix this issue. Let me know if you want to take a stab at it, otherwise, I'll try to find time upcoming weeks to make the code more generic and release a new version.

In the meantime, as a workaround, you could create your own CognitoProxyRequest that would support the structure you need. E.g.:

  case class CognitoProxyRequest[T](
      path: String,
      pathParameters: Option[Map[String, String]] = None,
      httpMethod: String,
      headers: Option[Map[String, String]] = None,
      queryStringParameters: Option[Map[String, String]] = None,
      stageVariables: Option[Map[String, String]] = None,
      requestContext: CognitoRequestContext = CognitoRequestContext(),
      body: Option[T] = None
  )

  case class CognitoRequestContext(
      authorizer: Option[CognitoRequestContextAuthorizer] = None
  )

  case class CognitoRequestContextAuthorizer(
      claims: ...
  )

If you go down this road, make sure to put all the classes into a separate package so that you could easily find and remove them later when the new version of the library is released.

Does this help?

dgiebert commented 3 years ago

Yes I am using a Cognito User pool.

This does help, but the main problem is to use the CognitoProxyRequest as suggested it would need to be accepted by the handle function. But as this only accepts a ProxyRequest type I would need to inherit from a case-class which is not possible in recent scala.

I think one approach could be to create a trait for the ProxyRequest something like ProxyRequestTrait to not break compatibility and inherit from it, so that others can overwrite the default behavior.

As this is quite a change in the code, I could take a look but would prefer if you could find some time ?

mkotsur commented 3 years ago

This does help, but the main problem is to use the CognitoProxyRequest as suggested it would need to be accepted by the handle function. But as this only accepts a ProxyRequest type I would need to inherit from a case-class which is not possible in recent scala.

In fact, Proxy is merely a very small wrapper around the vanilla Lambda type: type Proxy[I, O] = Lambda[ProxyRequest[I], ProxyResponse[O]]. So, you can easily redefine that too and instead of extending Proxy; assuming you've got the code similar to what I wrote in the previous comment, just do:

class MyCognitoReadyHandler extends Lambda[CognitoProxyRequest[I], ProxyResponse[O]] {
...
}

and that should be enough. Inheritance, indeed, doesn't play well with case classes in Scala. But in this situation, I wouldn't be worried too much about copy-pasting some fields.

I'm going to make the library more flexible anyway, has got some ideas about how to do that. Hope to release a new version either early next week.

mkotsur commented 3 years ago

Hey there,

I've released a snapshot version, which you can try doing this in build.sbt

resolvers +=
  "OSS snapshots" at "https://oss.sonatype.org/content/repositories/snapshots"
libraryDependencies += "io.github.mkotsur" %% "aws-lambda-scala" % "0.2.1-SNAPSHOT"

Then your l-function will look like:

import io.github.mkotsur.aws.handler.Lambda
import MyProxy._
import io.circe.generic.auto._
import Lambda._
import com.amazonaws.services.lambda.runtime.Context
import io.github.mkotsur.aws.proxy
import io.github.mkotsur.aws.proxy.ProxyResponse

object MyProxy {
  case class MyRequestBody()
  case class MyClaims(token_use: String, auth_time: Long /*, exp: Date, iat: Date */ )
  case class MyAuthorizer(claims: MyClaims)
  case class MyContext(authorizer: MyAuthorizer)
  case class MyResponseBody(foo: String)
}

class MyProxy extends Lambda.ApiProxy[MyRequestBody, MyContext, MyResponseBody]{
  override protected def handle(i: proxy.ApiProxyRequest[MyRequestBody, MyContext], c: Context): Either[Throwable, proxy.ProxyResponse[MyResponseBody]] = {

    // Do whatever you want with `i.requestContext.authorizer.claims` here
    Right(ProxyResponse.success(Some(MyResponseBody("Hello..."))))
  }
}

Note 1: you'll need an extra import import io.circe.generic.auto._ (or equivalent) if you do want to deserialize your context into case classes. Note 2: if you don't want to deserialize the context JSON into case classes, you can supply io.circe.Json as the second type parameter in Lambda.ApiProxy

Please test this and let me know if it works for you. The code changes are on the issue-24 branch.

dgiebert commented 3 years ago

Hey there,

this works for me, thanks ! :+1:

  case class CognitoClaims(
      sub: String,
      token_use: String,
      auth_time: Long,
      iss: String,
      exp: String,
      iat: String,
      email_verified: Option[String],
      phone_number_verified: Option[String],
      given_name: Option[String],
      `cognito:username`: Option[String],
      family_name: Option[String],
      phone_number: Option[String],
      email: Option[String],
      client_id: Option[String]
  )

Maybe it can added the instructions to the README, e.g. by pointing to the test case ?

mkotsur commented 3 years ago

Very cool. Thanks for the case class and the suggestion to mention this in the README. Great idea, I'll do that.

I also want to see if there is a (relatively) easy way to have a default type for MyContext. So that you would need to specify it in extends Lambda.ApiProxy[MyRequestBody, MyContext, MyResponseBody] only when you're interested in having your context represented as case classes.

And if you are not, you could do something like:

extends Lambda.ApiProxy[MyRequestBody, MyResponseBody]

and the context would still be available but as raw Json type.

Anyhow, will try to timebox this by the end of this week and make a normal release with or without this type-sugar.

mkotsur commented 3 years ago

Fixed in 0.3.0