Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Add Authoriser Based on User Pool Group #617

Merged
merged 10 commits into from
Oct 28, 2024

Conversation

williamputraintan
Copy link
Member

  • Add AWS Verified Permission to OrcaBus to handle authorisation requests.
  • Add lambda authoriser for API-GW to check if the token used is within the admin group of the user pool.

Depends on umccr/infrastructure#487

@@ -153,12 +153,14 @@ export class Filemanager extends Stack {
new HttpRoute(this, 'PatchHttpRoute', {
httpApi: httpApi,
integration: apiIntegration,
authorizer: apiGateway.cognitoAdminGroupAuthorizer,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the FM might be a use case where only the admin user (including orcabus_token) could do non-read methods (e.g. POST, PATCH). @mmalenic

Copy link
Member Author

@williamputraintan williamputraintan Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@victorskl Just an important note on each stage we must put the orcabus_token service on the admin Cognito group and refresh the service JWT rotator to get the latest token claim.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, will do. Thanks, Will.

Copy link
Member

@reisingerf reisingerf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Minor comment/fix.

lib/workload/components/api-gateway/README.md Outdated Show resolved Hide resolved
@victorskl
Copy link
Member

@williamputraintan Ok to merge?

@williamputraintan
Copy link
Member Author

williamputraintan commented Oct 28, 2024

@williamputraintan Ok to merge?

Yes, but we would probably need cognito in stg and prod to sync up. Happy if we want to leave this out from O3 ?

@victorskl
Copy link
Member

Yup, will sync up cognito accordingly, Will. No worries.

@williamputraintan
Copy link
Member Author

Ok, will merge after the testing of resolved conflict is finished.

@williamputraintan williamputraintan merged commit 8ee4366 into main Oct 28, 2024
6 checks passed
@williamputraintan williamputraintan deleted the feat/http-admin-authoriser- branch October 28, 2024 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants