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.

Sign up for GitHub

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

Jump to bottom

fix: retrieve internal user id instead and pass it to the UserSignIn audit log instead of using the Cognito sub id #3266

Merged
merged 3 commits into from
Feb 22, 2024
Merged

fix: retrieve internal user id instead and pass it to the UserSignIn audit log instead of using the Cognito sub id #3266

merged 3 commits into from
Feb 22, 2024

Conversation

craigzour
Copy link
Contributor

Summary | Résumé

closes #3264

  • Fixed issue where the wrong user id was used in the UserSignIn audit log. It used to be the Cognito sub id and it will now be our internal user id from PostgreSQL

@craigzour craigzour self-assigned this Feb 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

🧪 Review environment

https://ye7olbnfh7jtxnu5ybcbzw5kre0tjxvu.lambda-url.ca-central-1.on.aws/

wmoussa-gc
wmoussa-gc reviewed Feb 21, 2024
View reviewed changes
lib/users.ts Outdated Show resolved Hide resolved
@craigzour craigzour force-pushed the fix/user-sign-in-audit-log-has-wrong-user-id branch from 495cfa8 to d9cbd3f Compare February 21, 2024 14:24
@craigzour craigzour marked this pull request as draft February 21, 2024 15:20
bryan-robitaille
bryan-robitaille requested changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@bryan-robitaille bryan-robitaille left a comment

Choose a reason for hiding this comment

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

Let's remove the getOrCreateUser and replace it with a direct primsa call that fetches the user ID based on email. This ensures that the scope of the logging function can't create a user and can only read a user.

@craigzour
fix: retrieve internal user id instead and pass it to the UserSignIn …
b277be4 
…audit log instead of using the Cognito sub id
@craigzour craigzour force-pushed the fix/user-sign-in-audit-log-has-wrong-user-id branch from d9cbd3f to b277be4 Compare February 21, 2024 15:31
@craigzour craigzour marked this pull request as ready for review February 21, 2024 15:34
@craigzour craigzour merged commit 981336f into develop Feb 22, 2024
11 checks passed
@craigzour craigzour deleted the fix/user-sign-in-audit-log-has-wrong-user-id branch February 22, 2024 16:14
@sre-read-write sre-read-write bot mentioned this pull request Feb 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryan-robitaille bryan-robitaille bryan-robitaille approved these changes

@wmoussa-gc wmoussa-gc wmoussa-gc approved these changes

Assignees

@craigzour craigzour

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Audit Log - User Sign In event using wrong user identifier
3 participants
@craigzour @bryan-robitaille @wmoussa-gc