Add new auth function in parallel upgraded to dotnet 8 #67

adamtry · 2025-03-13T10:58:49Z

I modified Adams's deployment strategy for the following reasons:

We cannot redeploy the original 3.1 version, so there is little benefit in keeping the code. By changing the service name, a new lambda will be created and the existing one will remain
I didn't know what code had been modified when it had been copied, making this PR more difficult

Policies

Most of the policies were added manually via the console. I have added these policies to the Serverless template.

The deployment needs to work for both staging and production environments (there is a dev environment, but I don't think it's used). I've included the accountIds for the HousingProd and HousingStaging accounts.

custom:
  housingAccountIds:
    staging: "087586271961"
    production: "282997303675"

We need a policy for the authorizer to be called from each AWS account. The first policy is for the StagingAPIs or ProductionAPIs account. The second is for HousingStaging and HousingProduction. If there are more AWS accounts required, they will need to be added here.

    AllowApisAccountAuthorizer:
      # ProductionApis or StagingApis
      Type: AWS::Lambda::Permission
      Properties:
        FunctionName:
          Fn::GetAtt:
            - ApiAuthVerifyTokenNewLambdaFunction
            - Arn
        Action: lambda:InvokeFunction
        Principal: apigateway.amazonaws.com
        SourceArn:
          Fn::Join:
            - ""
            - - "arn:aws:execute-api:"
              - Ref: AWS::Region
              - ":"
              - Ref: AWS::AccountId
              - ":*"
    AllowHousingCrossAccountAuthorizer:
      # Housing-Production or Housing-Staging
      Type: AWS::Lambda::Permission
      Properties:
        FunctionName:
          Fn::GetAtt:
            - ApiAuthVerifyTokenNewLambdaFunction
            - Arn
        Action: lambda:InvokeFunction
        Principal: apigateway.amazonaws.com
        SourceArn:
          Fn::Join:
            - ""
            - - "arn:aws:execute-api:"
              - Ref: AWS::Region
              - ":"
              - ${self:custom.housingAccountIds.${self:provider.stage}}
              - ":*/authorizers/*"

Additionally, we need a policy for each account to assume the role when calling the GetRestApiAsync method in ApiGateway. This policy might be redundant if we remove the call to API Gateway in a later PR.

- PolicyName: assumeRoleForGettingCredentialsApiAccount
            PolicyDocument:
              Version: "2012-10-17"
              Statement:
                - Effect: Allow
                  Action:
                    - "sts:AssumeRole"
                  Resource:
                    Fn::Join:
                      - ""
                      - - "arn:aws:iam::"
                        - Ref: "AWS::AccountId"
                        - ":role/LBH_Api_Gateway_Allow_GET"
          - PolicyName: assumeRoleForGettingCredentialsHousingAccount
            PolicyDocument:
              Version: "2012-10-17"
              Statement:
                - Effect: Allow
                  Action:
                    - "sts:AssumeRole"
                  Resource:
                    Fn::Join:
                      - ""
                      - - "arn:aws:iam::"
                        - ${self:custom.housingAccountIds.${self:provider.stage}}
                        - ":role/LBH_Api_Gateway_Allow_GET"

Updating the ValidateToken Method

When testing the ValidateToken method, I repeatedly encountered the following error IDX10503: Signature validation failed. Token does not have a kid. Keys tried: .... I'm unsure if this change was due to updating the JWT library or upgrading to DOTNET 8, but I could not find a workaround. Our JWT tokens don't contain a KID header, and there was no way to disable the check without turning off signature verification entirely.

For that reason, I've taken the approach to use the JWT library (https://www.nuget.org/packages/JWT). It enables us to verify the token in the same way, but without requiring the missing KID header.

Previous PR Description

Context: We need to update the auth-verify-token function to dotnet 8 due to a deadline where dotnet 6 lambda functions will no longer be deployable on AWS. We will need this function to be deployable in a short time-frame so that we can fix a timeout issue in the VerifyAccessUseCase _awsApiGateway.GetApiName that is suspected to be surpassing account-level usage limits.

This verify-token function is used by many of our APIs across multiple AWS accounts, so any errors could cause serious widespread disruption. For this reason it was decided to deploy the dotnet 8 version of this token verifier in parallel with the existing one, and phasing the old one out more carefully after testing with the new one in a limited way.

The main Files changed will be somewhat difficult to read through because the original ApiAuthVerifyToken and ApiAuthVerifyToken.Tests folders were copied into new folders ApiAuthVerifyTokenNew and ApiAuthVerifyTokenNew.Tests so that we can deploy the dotnet 8 version of this function in parallel and keep the original deployment and tests as-is.

After the dotnet 8 function is in production and well-tested, it's assumed that a task will be taken to remove it from this code base. The new solutions are parallel to the existing ones so that the old should be removable without affecting the new.

~~See a7619bd for the actual changes to get dotnet 8 and parallel deployment working~~

~~The dotnet 8 upgrade steps were mostly copied from an existing dotnet 8 upgrade PR that was before we decided to deploy in parallel: #65 - see this for further comments and explanations~~

Note: It is expected that the existing 3.1 deployment will fail because deploying 3.1 isn't supported anymore, but at least it shouldn't try to destroy the existing function. The dotnet 8 deployment is in parallel with this in the CircleCI workflow so should not be blocked

adamtry · 2025-03-13T15:41:43Z

.circleci/config.yml

+    steps:
+      - deploy-lambda:
+          stage: "staging"
+          path: "./ApiAuthVerifyTokenNew/"


Only deploy the new one to staging for now. I didn't want to risk accidental production deployments when I don't yet know how it will interact with other infrastructure

adamtry · 2025-03-13T15:44:58Z

ApiAuthVerifyTokenNew.Tests/V1/AcceptanceTests/VerifyTokenAcceptanceTests.cs

-            Environment.SetEnvironmentVariable("jwtSecret", _fixture.Create<string>());
-            Environment.SetEnvironmentVariable("hackneyUserAuthTokenJwtSecret", _faker.Random.AlphaNumeric(25));
+            Environment.SetEnvironmentVariable("jwtSecret", _faker.Random.AlphaNumeric(50));
+            Environment.SetEnvironmentVariable("hackneyUserAuthTokenJwtSecret", _faker.Random.AlphaNumeric(50));


This is due to the new requirements in dotnet 8 that these secrets must be longer. See https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/wiki/IDX10720

martapederiva · 2025-03-13T16:12:43Z

@LBHCallumM

LBHCallumM · 2025-03-13T16:23:56Z

@LBHCallumM

:(

Duslerke

If you want a temporary test (because let's face it - that's what it is - a spike to see whether the upgrade would work), you don't need to commit this to master branch source control.

Just do these changes to the existing project files (no need for dupes) on a spike dummy/test branch where you'll tweak the pipeline to deploy from your spike branch instead of master.
Make sure to change the CF stack name in the serverless file so it gets deployed as a separate stack.
Deploy a temp version of dotnet 8 lambda under a new stack.
Once you're done testing, you can raise the PR from your spike branch onto the main branch after you've tweaked back the pipeline to deploy as normal.

This avoids the messy git history, excessive duplicated files, hard-to-read PR diff, and potentially forever lingering confusing suffixes like -new here or -2 in housing-repairs-online-2.

Lastly, if you'll notice the CircleCI pipeline, to deploy to development environment, you need the development branch. Your PR is targeting master. You should fix the development branch first and merge whatever PR to this branch first, otherwise you'd be skipping updating development environment and going straight to staging. Huge nono.

Doing 'request changes' only to pull some attention to the comment given 2 approvals are already present.

Duslerke

Approving to unblock it because of reported lack of time and will for this to be re-done via the proper approach mentioned in the previous review (as this piece of work is taken over due to original author being on leave).

Duslerke

Seems alright at a glance.
Nice switch to a different JWT.

Duslerke · 2025-03-21T09:41:30Z

.circleci/config.yml

+  check:
+    jobs:
+      - check-code-formatting:
+          filters:
+            branches:
+              ignore: master
+      - build-and-test:
+          filters:
+            branches:
+              ignore: master


ignore master and development

Duslerke · 2025-03-21T09:42:57Z

.circleci/config.yml

+
+  temp-deploy-new-staging-for-testing-purposes:
+    jobs:
+      - build-and-test
+      - assume-role-staging:
+          context: api-assume-role-staging-context
+          requires:
+            - build-and-test
+      - permit-staging-release:
+          type: approval
+          requires:
+            - assume-role-staging
+      - terraform-init-and-apply-to-staging:
+          requires:
+            - permit-staging-release
+      - deploy-to-staging:
+          context:
+            - "Serverless Framework"
+          requires:
+            - terraform-init-and-apply-to-staging


Is this PR for this temp deploy, are you planning to move onto live now?
Wondering whether this configuration is something leftover from your testing.

Duslerke · 2025-03-21T09:45:39Z

ApiAuthVerifyToken.sln

 EndProject
-Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ApiAuthVerifyToken.Tests", "ApiAuthVerifyToken.Tests\ApiAuthVerifyToken.Tests.csproj", "{DE8E4E61-EB76-416B-8417-E8F2F6A0C0D3}"
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ApiAuthVerifyToken.Tests", "ApiAuthVerifyToken.Tests\ApiAuthVerifyToken.Tests.csproj", "{DE8E4E61-EB76-416B-8417-E8F2F6A0C0D3}"


The project GUID somehow changed?

adamtry · 2025-03-24T13:07:32Z

We cannot redeploy the original 3.1 version, so there is little benefit in keeping the code. By changing the service name, a new lambda will be created and the existing one will remain
I didn't know what code had been modified when it had been copied, making this PR more difficult
Policies
Most of the policies were added manually via the console. I have added these policies to the Serverless template.

The deployment needs to work for both staging and production environments (there is a dev environment, but I don't think it's used). I've included the accountIds for the HousingProd and HousingStaging accounts.

custom:
housingAccountIds:
staging: "087586271961"
production: "282997303675"
We need a policy for the authorizer to be called from each AWS account. The first policy is for the StagingAPIs or ProductionAPIs account. The second is for HousingStaging and HousingProduction. If there are more AWS accounts required, they will need to be added here.

AllowApisAccountAuthorizer:
  # ProductionApis or StagingApis
  Type: AWS::Lambda::Permission
  Properties:
    FunctionName:
      Fn::GetAtt:
        - ApiAuthVerifyTokenNewLambdaFunction
        - Arn
    Action: lambda:InvokeFunction
    Principal: apigateway.amazonaws.com
    SourceArn:
      Fn::Join:
        - ""
        - - "arn:aws:execute-api:"
          - Ref: AWS::Region
          - ":"
          - Ref: AWS::AccountId
          - ":*"
AllowHousingCrossAccountAuthorizer:
  # Housing-Production or Housing-Staging
  Type: AWS::Lambda::Permission
  Properties:
    FunctionName:
      Fn::GetAtt:
        - ApiAuthVerifyTokenNewLambdaFunction
        - Arn
    Action: lambda:InvokeFunction
    Principal: apigateway.amazonaws.com
    SourceArn:
      Fn::Join:
        - ""
        - - "arn:aws:execute-api:"
          - Ref: AWS::Region
          - ":"
          - ${self:custom.housingAccountIds.${self:provider.stage}}
          - ":*/authorizers/*"

Additionally, we need a policy for each account to assume the role when calling the GetRestApiAsync method in ApiGateway. This policy might be redundant if we remove the call to API Gateway in a later PR.

PolicyName: assumeRoleForGettingCredentialsApiAccount
PolicyDocument:
Version: "2012-10-17"
Statement:
- Effect: Allow
Action:
- "sts:AssumeRole"
Resource:
Fn::Join:
- ""
- - "arn:aws:iam::"
- Ref: "AWS::AccountId"
- ":role/LBH_Api_Gateway_Allow_GET"
- PolicyName: assumeRoleForGettingCredentialsHousingAccount
PolicyDocument:
Version: "2012-10-17"
Statement:
- Effect: Allow
Action:
- "sts:AssumeRole"
Resource:
Fn::Join:
- ""
- - "arn:aws:iam::"
- ${self:custom.housingAccountIds.${self:provider.stage}}
- ":role/LBH_Api_Gateway_Allow_GET"
Updating the ValidateToken Method
When testing the ValidateToken method, I repeatedly encountered the following error IDX10503: Signature validation failed. Token does not have a kid. Keys tried: .... I'm unsure if this change was due to updating the JWT library or upgrading to DOTNET 8, but I could not find a workaround. Our JWT tokens don't contain a KID header, and there was no way to disable the check without turning off signature verification entirely.

For that reason, I've taken the approach to use the JWT library (https://www.nuget.org/packages/JWT). It enables us to verify the token in the same way, but without requiring the missing KID header.

Previous PR Description
Context: We need to update the auth-verify-token function to dotnet 8 due to a deadline where dotnet 6 lambda functions will no longer be deployable on AWS. We will need this function to be deployable in a short time-frame so that we can fix a timeout issue in the VerifyAccessUseCase _awsApiGateway.GetApiName that is suspected to be surpassing account-level usage limits.

This verify-token function is used by many of our APIs across multiple AWS accounts, so any errors could cause serious widespread disruption. For this reason it was decided to deploy the dotnet 8 version of this token verifier in parallel with the existing one, and phasing the old one out more carefully after testing with the new one in a limited way.

The main Files changed will be somewhat difficult to read through because the original ApiAuthVerifyToken and ApiAuthVerifyToken.Tests folders were copied into new folders ApiAuthVerifyTokenNew and ApiAuthVerifyTokenNew.Tests so that we can deploy the dotnet 8 version of this function in parallel and keep the original deployment and tests as-is.

After the dotnet 8 function is in production and well-tested, it's assumed that a task will be taken to remove it from this code base. The new solutions are parallel to the existing ones so that the old should be removable without affecting the new.

See a7619bd for the actual changes to get dotnet 8 and parallel deployment working

The dotnet 8 upgrade steps were mostly copied from an existing dotnet 8 upgrade PR that was before we decided to deploy in parallel: #65 - see this for further comments and explanations

Note: It is expected that the existing 3.1 deployment will fail because deploying 3.1 isn't supported anymore, but at least it shouldn't try to destroy the existing function. The dotnet 8 deployment is in parallel with this in the CircleCI workflow so should not be blocked

adamtry added 6 commits March 12, 2025 16:11

Docker space compose

2b2dab5

Fix makefile

4cc4470

Copy into new folders

ad0eecb

Dotnet 8 parallel function

a7619bd

Simplify maybe

9b06dc5

Keep build

0386957

adamtry commented Mar 13, 2025

View reviewed changes

adamtry marked this pull request as ready for review March 13, 2025 15:46

adamtry requested a review from a team as a code owner March 13, 2025 15:46

adamtry changed the title ~~Upgrade to dotnet8 parallel~~ Add new auth function in parallel with dotnet 8 Mar 13, 2025

adamtry changed the title ~~Add new auth function in parallel with dotnet 8~~ Add new auth function in parallel upgraded to dotnet 8 Mar 13, 2025

LBHCallumM previously approved these changes Mar 13, 2025

View reviewed changes

martapederiva previously approved these changes Mar 13, 2025

View reviewed changes

Duslerke requested changes Mar 14, 2025

View reviewed changes

Fix namespaces

6349016

Duslerke previously approved these changes Mar 17, 2025

View reviewed changes

LBHCallumM dismissed stale reviews from Duslerke, martapederiva, and themself via 5fdc60d March 17, 2025 10:30

LBHCallumM added 8 commits March 17, 2025 10:30

Add new projects to solution

5fdc60d

update pipelihne

4ee28e9

update dockerfile

8bd6371

Fix dependency issues

923eb5d

DF

6d23b1b

remove 'new' projects

dbae2c1

update config

2d224d2

update config

128143a

LBHCallumM added 11 commits March 19, 2025 12:36

make authorizer permission more permissive

0190e2e

attempt 2

58c08a0

arn

e0cba13

remove unused policy

2e44962

Speed up pipeline

f68fc78

re-add permission for fetching credentials from housing-staging

20db902

switch back to dynamic sourceArn

2a7230c

change cross-account policy names to be more generic

b80a48b

cleanup

ef51f57

cleanup config.yml

81e8926

add missed filters

574d6c4

Duslerke previously approved these changes Mar 21, 2025

View reviewed changes

LBHCallumM dismissed Duslerke’s stale review via 196be30 March 21, 2025 10:32

LBHCallumM and others added 9 commits March 21, 2025 10:33

Add DR account policies

196be30

fix serverless error

16649f3

toggle policy

191fa48

missed comment

386f291

Update serverless condition syntax

59351e0

Add back other DR policy

24d1063

extract DR policy logic

7ad0a35

Remove auto staging deploy

d517002

Check ignore master and development

634755b

adamtry changed the base branch from master to development March 24, 2025 12:44

adamtry requested a review from a team March 24, 2025 12:56

humulla approved these changes Mar 24, 2025

View reviewed changes

Miles-Alford approved these changes Mar 24, 2025

View reviewed changes

adamtry merged commit 8ffe58d into development Mar 24, 2025
3 checks passed

adamtry deleted the upgrade-to-dotnet8-parallel branch March 27, 2025 11:28

adamtry mentioned this pull request Mar 27, 2025

Deploy the dotnet8 upgraded auth-verify-token function in parallel #72

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add new auth function in parallel upgraded to dotnet 8 #67

Add new auth function in parallel upgraded to dotnet 8 #67

Uh oh!

adamtry commented Mar 13, 2025 •

edited by LBHCallumM

Loading

Uh oh!

adamtry Mar 13, 2025

Uh oh!

adamtry Mar 13, 2025

Uh oh!

martapederiva commented Mar 13, 2025

Uh oh!

LBHCallumM commented Mar 13, 2025

Uh oh!

Duslerke left a comment •

edited

Loading

Uh oh!

Duslerke left a comment

Uh oh!

Duslerke left a comment

Uh oh!

Duslerke Mar 21, 2025

Uh oh!

Duslerke Mar 21, 2025

Uh oh!

Duslerke Mar 21, 2025

Uh oh!

adamtry commented Mar 24, 2025

Uh oh!

Uh oh!

Uh oh!

Add new auth function in parallel upgraded to dotnet 8 #67

Add new auth function in parallel upgraded to dotnet 8 #67

Uh oh!

Conversation

adamtry commented Mar 13, 2025 • edited by LBHCallumM Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Policies

Updating the ValidateToken Method

Previous PR Description

Uh oh!

adamtry Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

adamtry Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

martapederiva commented Mar 13, 2025

Uh oh!

LBHCallumM commented Mar 13, 2025

Uh oh!

Duslerke left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Duslerke left a comment

Choose a reason for hiding this comment

Uh oh!

Duslerke left a comment

Choose a reason for hiding this comment

Uh oh!

Duslerke Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

Duslerke Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

Duslerke Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

adamtry commented Mar 24, 2025

Uh oh!

Uh oh!

Uh oh!

adamtry commented Mar 13, 2025 •

edited by LBHCallumM

Loading

Duslerke left a comment •

edited

Loading