INCIDEN-922: further access control #906
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ryan-Andrews99
force-pushed
the
inciden-922-further-access-control
branch
from
90f1fc3 to
d223a35
Compare
Ryan-Andrews99
had a problem deploying
to
demo
GitHub Actions
Failure
Ryan-Andrews99
had a problem deploying
to
demo
GitHub Actions
Failure
Ryan-Andrews99
force-pushed
the
inciden-922-further-access-control
branch
from
d223a35 to
51a0eba
Compare
Ryan-Andrews99
had a problem deploying
to
demo
GitHub Actions
Failure
Ryan-Andrews99
force-pushed
the
inciden-922-further-access-control
branch
from
51a0eba to
a9e7177
Compare
Our lambda on the backend will now expect an access token, so we need to start passing it in the GET request. We already validate the token so this is just a case of updating the interface and ensuring we pass it through
Ryan-Andrews99
force-pushed
the
inciden-922-further-access-control
branch
from
795f3a5 to
9a0a7bc
Compare
We have purposely left getDynamoDBEntriesHandler handler without authorisation as it is called when a useer may not have an access token present as part of the forgot password flow. I have also removed including the userEmail in the error message as this is unneccesary
These lambdas are called by a step function, and the authorisation happens before they are called. This means we do not need to implment additonal authorisation here
This checks that the user Id included in the request body matches the userId in the access token
The updateUser method does not use the cognitoUserId argument, and it is redundant as the userId provided is the cognitoUserId with a `user#` prefix. This also makes the lambda code less confusing as where we call the `/update-user` endpoint the body does not include this value.
This checks that the userId in the access token matches the userId in the update request body, ensuring a user can only update their own entries
We're now expecting an access token on the backend, so we need to start passing it through here not to break any existing functionality.
Ryan-Andrews99
force-pushed
the
inciden-922-further-access-control
branch
from
9a0a7bc to
ef5c25a
Compare
CarlyG55
approved these changes
Sep 18, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Ryan-Andrews99
temporarily deployed
to
development
GitHub Actions
Inactive
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Onboarding Feature Deployment
Warning
Pull requests merged to
mainwill be released to production, please ensure the checklist below is completeBefore any work can be merged to main in must meet the definition of done and be ready to deploy. While many of these tasks will be automated, the reviewers must take the responsibility of confirming the checklist below has been completed before this ticket can be merged.
Checklist
this pull request meets the acceptance criteria of the ticket
this branch is up-to-date with the main branch
git fetch --all && git rebase origin/mainthese changes are backwards compatible (no breaking changes)
@deprecatedtests have been written to cover any new or updated functionality
new configuration parameters have been deployed to all environments, see configuration management.
all external infrastructure dependencies have been updated in all environments
Changes
[ please list the changes this pull request is making ]
Addedfor new featuresChangedfor changes in existing functionalityDeprecatedfor soon-to-be removed featuresRemovedfor now removed featuresFixedfor any bug fixesSecurityin case of vulnerabilities