-
Notifications
You must be signed in to change notification settings - Fork 0
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
Pentest #1566
Pentest #1566
Conversation
Endpoint URL - https://d108315ije1eu7.cloudfront.net |
return false; | ||
} | ||
|
||
const allowedRoles = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for turning this into a whitelist of allowed roles!
@@ -106,6 +106,7 @@ const UserManagement = () => { | |||
console.log("Error while fetching user list.", error); | |||
setAlertCode(RESPONSE_CODE[error.message]); | |||
}); | |||
// eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please discard this change as no functional changes were made to this file
@@ -161,7 +161,7 @@ const UserPage = () => { | |||
const { userProfile, setUserInfo, updatePhoneNumber, userRole, userStatus } = | |||
useAppContext(); | |||
const location = useLocation(); | |||
const { userId } = useParams() ?? {}; | |||
let { userId } = useParams() ?? {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious why this change was made........might even be flagged by lint if setting with let but is never overridden.....but also probably userId is something we dont want to let get overwritten?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right I did this when I was considering how to encrypt the email/userId before Andy fixed it
Story: https://jiraent.cms.gov/browse/OY2-31468
https://jiraent.cms.gov/browse/OY2-31474
Endpoint: https://d108315ije1eu7.cloudfront.net/
Details
added a batch function that modifies the idToken JWT with a "custom:user_roles" attribute with the roles retrieved for each user from dynamo.
Added route protection for the /usermanagement route which checks the jwt for proper role access
Changes
-also added the trigger function for potential use if the intermittent functionality problem is fixed.
Test Plan
Just a smoke test to make sure nothing is broken, The pen testers will need to verify that the usermanagent screen cannot be accessed by modifying the response to UserDataApi.userProfile by setting the admin role status from "revoked" to "active"