-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
Create groups for ptokens #7944
Conversation
print('Adding seeded users to pToken Group') | ||
|
||
# TODO: How else will we determine which users should be added to the group | ||
valid_users = User.objects.filter(~Q(groups__name__in=[pToken_group_name]), profile__trust_profile=True) |
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.
Just need clarity on the identifier to seed the initial user base. Email / ids ? @thelostone-mc
@@ -204,6 +204,12 @@ def ptoken(request, token_id='me'): | |||
return JsonResponse( | |||
{'error': _('You must be authenticated via github to use this feature!')}, | |||
status=401) | |||
|
|||
if not user.has_perm('auth.user.add_pToken_auth'): |
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.
@mds1 Brought up the good point that a user would be able to abuse Vue here and possibly open the modal; this ensures that they cannot make the request given their creds
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.
LGTM
b380f56
to
f1806a8
Compare
for user in valid_users: | ||
user.groups.add(pToken_group) | ||
|
||
print('Valid users added to pToken group') |
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.
Do we run this just once and be done with it ?
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.
yup, unless you had another way in mind. Let me know how seeding is determined and I can open another PR and add it to this command
f1806a8
to
96a5903
Compare
Just tested this ! @mds1 @corydickson |
I'll defer to @corydickson on this one. From a quick glance it looks like it's set up correctly, where the "Create" button is behind Can we also add instructions/comments somewhere (even if just as a reply here) that explain what |
Description
Returns a 403 on the create ptoken endpoint if a user does not have the necessary permissions and hides the model the dashboard page.
Testing
Run the
ptoken
migration to add a user to the group and create the necessary permissions. Then, login and navigate to the dashboard page. If a user has permission should see the modal, else it will not appear