Upgrade AWS IAM v1 client to v2

[kelvin-chappell]

What is the purpose of this change?

This change removes all dependence on AWS v1 SDKs and on the archived awscala library.
All interaction with AWS is now done through v2 SDKs.

What is the value of this change and how do we measure success?

This will make the code more secure and easier to maintain.

Any additional notes?

AWS provides a library for modelling IAM policies but to make use of this library would involve changing a lot of code in this repo and in the call sites of the configTools library, making the change history difficult to navigate.
Instead we have created case classes to model policies that duplicate the pre-existing awscala case classes.
This has the benefit of only needing to change imports in most cases so preserving the change history. But it comes at the cost of having to generate a json representation of policies, which is more complicated than just auto-encoding the case class structure.

To test

Verify in production that:

• Can still sign into AWS console from Janus
• Can sign out from console and back in again from Janus
• Can retrieve credentials and use them in command line locally
• Can revoke all credentials for an account and see policy condition has correct threshold time and console and local credentials unusable
• Logs have no new errors

See also

• Remove v1 AWS dynamodb client and awscala dynamo lib #500
• Upgrade AWS STS client to v2 #504

[gu-scala-library-release]

@kelvin-chappell has published a preview version of this PR with release workflow run #2, based on commit aa8d44a:

2.0.0-PREVIEW.kciam-upgrade.2025-02-13T1423.aa8d44ad

Want to make another preview release?

Click 'Run workflow' in the GitHub UI, specifying the kc/iam-upgrade branch, or use the GitHub CLI command:

gh workflow run release.yml --ref kc/iam-upgrade

Want to make a full release after this PR is merged?

Click 'Run workflow' in the GitHub UI, leaving the branch as the default, or use the GitHub CLI command:

gh workflow run release.yml

[tjsilver]

Should we use Java time now instead?

[kelvin-chappell]

Yes we should! I'm working on that on another branch

[tjsilver]

Have tested locally and all the expectations listed were met.

[adamnfish]

Looks good from a reading the code point of view! I haven't yet actually exercised these bits locally.

As well as testing that the Janus webapp still works, it'd be great to know for sure we aren't affecting the Janus Data config format that will be produced by consumers of the config-tools library. One way to do that would be to take an example configuration generated both before and after this change. We can (privately) use our real Guardian config of course, but another candidate is the "example" project in this repository. This sub-project doesn't get built in CI/CD and I think should probably go away completely, but while it exists maybe that can serve as a useful sample config so we can check in a reference file during this transition period.

I think the config file serialisation behaviour is as key a guarantee for us to preserve as Janus' actual end-user behaviour is. We'd like the existing and new Janus Data files to be readable by the existing and new versions of the code, so that the migration (and reversion) path is as smooth as possible. Whenever we have an important property of the code like this, ideally we have that captured in a test!

Note: heads-up that it looks like comparing the current and previous Janus Data files will result in statement ID differences, as I mention below.

[adamnfish]

🥳

[adamnfish]

Since this test is checking the entire policy, should we assert the complete JSON structure? I'm not certain!

I do have a slight sense that if we're going to check each element separately we could do that with a separate test for each of these "properties". The - / in syntax is great for this, because it lets you share common setup and then do a bunch of independent minimal checks.

"should JSON encode complex policy correctly" - {
  val complexPolicy = ...
  
  "with correct version" in {
    val json = complexPolicy.asJson.noSpaces
    json should include(""""Version":"2012-10-17"""")
  }

  "with correct ID" in {
    val json = complexPolicy.asJson.noSpaces 
    json should include(""""Version":"2012-10-17"""")
  }
}

Having these separate elements tested individually is great (helps point the developer at the actual source of the problem), but no harm in also testing the whole JSON string itself as well, as mentioned above.

You could probably share the val json = complexPolicy.asJson.noSpaces step in this case if you want to as well, but generally speaking you don't want to share the actual operation you're testing - it should be duplicated in each test. The reason is that if it throws an exception outside a test block your test suite will fail horribly, you only want to run application code from within an in block.

[kelvin-chappell]

Good point. I'll break down the tests and add a full json test.

[kelvin-chappell]

Done in abfb417

[adamnfish]

Do we need this Iam object wrapper? No harm, I can see (minor) arguments both ways 👍

[kelvin-chappell]

It seemed like a convenient reference to hold everything together. But can remove if you prefer - not strictly necessary.

[adamnfish]

Let's talk about Statement IDs! The current implementation adds unique SIDs using incrementing integers, so if we're going for compatibility we should keep that behaviour. However, for inline policies (which is what Janus produces) SIDs are not required and I was already considering removing them.

Let's talk about the behaviour we want for now in this PR and in the future. However, this does tie into my other point about having a test that ensures we are maintining Janus Data config-file-compatibility during this change.

[kelvin-chappell]

On the face of it, removing the statement IDs looks sensible. I'm not sure where or how they're referenced. Let's discuss next week.

[kelvin-chappell]

Now statement IDs will be included in the output json representation if they're explicitly added to a Statement case class, otherwise they'll be omitted.

[kelvin-chappell]

As well as testing that the Janus webapp still works, it'd be great to know for sure we aren't affecting the Janus Data config format that will be produced by consumers of the config-tools library. One way to do that would be to take an example configuration generated both before and after this change. We can (privately) use our real Guardian config of course, but another candidate is the "example" project in this repository. This sub-project doesn't get built in CI/CD and I think should probably go away completely, but while it exists maybe that can serve as a useful sample config so we can check in a reference file during this transition period.

I think the config file serialisation behaviour is as key a guarantee for us to preserve as Janus' actual end-user behaviour is. We'd like the existing and new Janus Data files to be readable by the existing and new versions of the code, so that the migration (and reversion) path is as smooth as possible. Whenever we have an important property of the code like this, ideally we have that captured in a test!

Note: heads-up that it looks like comparing the current and previous Janus Data files will result in statement ID differences, as I mention below.

I'll set this up and probably remove the statement IDs if they're redundant.

[kelvin-chappell]

As well as testing that the Janus webapp still works, it'd be great to know for sure we aren't affecting the Janus Data config format that will be produced by consumers of the config-tools library. One way to do that would be to take an example configuration generated both before and after this change. We can (privately) use our real Guardian config of course, but another candidate is the "example" project in this repository. This sub-project doesn't get built in CI/CD and I think should probably go away completely, but while it exists maybe that can serve as a useful sample config so we can check in a reference file during this transition period.

I think the config file serialisation behaviour is as key a guarantee for us to preserve as Janus' actual end-user behaviour is. We'd like the existing and new Janus Data files to be readable by the existing and new versions of the code, so that the migration (and reversion) path is as smooth as possible. Whenever we have an important property of the code like this, ideally we have that captured in a test!

Note: heads-up that it looks like comparing the current and previous Janus Data files will result in statement ID differences, as I mention below.

I'll set this up and probably remove the statement IDs if they're redundant.

A comparison of the data file output from the example project using v1.0.0 of configTools against the latest snapshot version of configTools showed that the output was identical. (Once I had reformatted, removed statement IDs and manually reordered object attributes). ie. all keys are the same and json without statement IDs is identical.