-
Notifications
You must be signed in to change notification settings - Fork 0
[stripe] hardcoded admin env vars #16
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
Conversation
/** | ||
* Helper function to check if a user is a hardcoded admin (username-only) | ||
* This function should be used consistently across authentication strategies | ||
* |
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.
nit: Maybe add something to comment like:
e.g. HARDCODED_ADMIN_USERNAMES=colinlin,mattmueller
} | ||
|
||
if (isHardcodedAdmin(user)) { | ||
user.role = SystemRoles.ADMIN; |
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.
Should we log here as well? It seems like checkAdminAccess is logged, so might be double-logging in the normal case, but we also use this function directly below
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.
kept user null check and changed isHardcodedAdmin to accept a username so no redundant null check logs
a1d7a40
to
25f2160
Compare
25f2160
to
76e0c2a
Compare
Pull Request Template
Summary
Please provide a brief summary of your changes and the related issue. Include any motivation and context that is relevant to your changes. If there are any dependencies necessary for your changes, please list them here.
Change Type
Please delete any irrelevant options.
Testing
Please describe your test process and include instructions so that we can reproduce your test. If there are any important variables for your testing configuration, list them here.
Test Configuration:
Checklist
Please delete any irrelevant options.