paws-r / paws

Paws, a package for Amazon Web Services in R
https://www.paws-r-sdk.com
Other
313 stars 37 forks source link

Potential issue with paws 0.4.0 AssumeRoleWithWebIdentity behavior #675

Closed lianos closed 11 months ago

lianos commented 11 months ago

I think this recently merged PR that enables EKS auth via AssumeRoleWithWebIdentity might not be 100% just yet.

I can provide more detailed debuging/source-tracing information of the fix I'm proposing, but I'll try to cut right to the chase first.

Also, apologies in advance if I'm misusing the terminology, or being a bit vague/obtuse. I'm not an expert on EKS or AWS IAM stuff, I'm just feeling around in the dark here a bit.

Background

We are using SaturnCloud, which also deploys analysis instances for end users via EKS. Users assume IAM roles on these instances so that they can interact with the rest of our AWS infra "as perscribed."

These roles are assigned using the whole ASSUME_ROLE_WITH_WEB_IDENTITY and AWS_WEB_IDENTITY_TOKEN_FILE mojo, and things were working swimmingly with paws 0.3.0 for us.

When paws 0.4.0 came out, we've found that the users are now assuming the role of the underlyling instance as opposed to the the webidentity that worked in the past.

For example, with paws 0.3.0 the paws::sts()$get_caller_identity() call matched the caller identity returned from the aws cli:

R> packageVersion("paws")
[1] '0.3.0'

R> paws::sts()$get_caller_identity()
$UserId
[1] "AROAQH6ZRAUT23O2GZDRN:default"

$Account
[1] "XXXXXXXXXXX"

$Arn
[1] "arn:aws:sts::XXXXXXXXXXX:assumed-role/XXX-saturn-data-analysts/default"

and from the shell:

$ aws sts get-caller-identity
{
    "UserId": "AROAQH6ZRAUT23O2GZDRN:botocore-session-XXXXXXX",
    "Account": "XXXXXXXXXXX",
    "Arn": "arn:aws:sts::XXXXXXXXXXX:assumed-role/XXX-saturn-data-analysts/botocore-session-XXXXXXX"
}

However when I upgrade to paws 0.4.0 on this same instance, we now get this:

R> packageVersion("paws")
[1] '0.4.0'

R> paws::sts()$get_caller_identity()
$UserId
[1] "AROAQH6ZRAUTWSHFSFYCA:i-0c346d13d2b4fb0f8"

$Account
[1] "XXXXXXXXXXX"

$Arn
[1] "arn:aws:sts::XXXXXXXXXXX:assumed-role/saturn-cluster-xxxx20230502032029935200000006/i-0c346d13d2b4fb0f8"

Proposed Superficial(?) Fix

I went digging down the paws::sts()$get_caller_identity() call stack to see what has changed between the behavior in paws 0.3.0 vs 0.4.0, and have discovered that the structure of the resp object returned from svc$assume_role_with_web_identity in the get_assume_role_with_web_identity_creds function has changed.

In v0.3.0, this call:

resp <- svc$assume_role_with_web_identity(RoleArn = role_arn,
    RoleSessionName = role_session_name, WebIdentityToken = web_identity_token)

generates a resp list, with "top-level" resp$Credentials and resp$AssumedRoleUser lists that has values at their top-level that can be directly accessed downstream.

What I mean is that calling resp$Credentials$AccessKeyId and resp$AssumedRoleUser$Arn gives you values to work with in paws 0.3.0 (paws.common 0.5.8)

However, in paws 0.4.0 (paws.common 0.6.0), the same resp object has a list of length-1 in resp$Credentials and resp$AssumedRoleUser.

So, instead of resp$Credentials$AccessKeyId providing you a valid value, you now have to call it as resp$Credentials[[1]]$AccessKeyId, and resp$AssumedRoleUser[[1]]$Arn to get the values you need.

I've found that if I simply hotwire the get_creds_from_sts_resp() function to pull out the first element of the Credentials object and use that, like so:

get_creds_from_sts_resp <- function(resp) {
  resp$Credentials <- resp$Credentials[[1]]
  role_creds <- Creds(
    access_key_id = resp$Credentials$AccessKeyId,
    secret_access_key = resp$Credentials$SecretAccessKey,
    session_token = resp$Credentials$SessionToken,
    expiration = as_timestamp(resp$Credentials$Expiration, "iso8601")
  )
  return(role_creds)
}

The role I want to assume on the instance is now recovered:

# after installing my hotwired paws.common package
R> paws::sts()$get_caller_identity()
$UserId
[1] "AROAQH6ZRAUT23O2GZDRN:default"

$Account
[1] "XXXXXXXXXXX"

$Arn
[1] "arn:aws:sts::XXXXXXXXXXX:assumed-role/XXX-saturn-data-analysts/default"

I know this isn't the right fix to move forward, but just wanted to point out what works now and hopefully help illuminate where we might go to debug further and help to find a more principled fix.

Thanks!

DyfanJones commented 11 months ago

Sorry about that, I will have a look at what is possibly causing this issue. My initial thought is the new xml_parse method. But I thought I got all scenarios 🤔

DyfanJones commented 11 months ago

I believe I have fixed this issue. Please feel free to try out paws.common 0.6.1.

remotes::install_github("DyfanJones/paws/paws.common", ref = "handlers_query")

If you are unable to compile please let me know and I will update r-universe so you can test it on your platform before it is released to the cran.

lianos commented 11 months ago

Sweet! I can confirm that this fix does the trick. Thanks for the quick response!

DyfanJones commented 11 months ago

paws.common 0.6.1 has been released to the cran. If this issue crops up again please feel free to re-open this ticket.