Skip to content

Commit

Permalink
Fixed user email auto verification on accepting organization invitati…
Browse files Browse the repository at this point in the history
…on (cvat-ai#7073)

There is a couple of UX bugs in invite user to organization feature.
This pr fixes:
- Email is auto-verified after accepting invitation
- Stuff can view unaccepted invitations
- Stuff can edit unaccepted memberships
- User email is now used as username
  • Loading branch information
klakhov committed Nov 3, 2023
1 parent 0535d45 commit 9819e6d
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 5 deletions.
2 changes: 1 addition & 1 deletion cvat-ui/src/components/organization-page/member-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function MemberItem(props: Props): JSX.Element {
const { username, firstName, lastName } = user;
const { username: selfUserName } = useSelector((state: CombinedState) => state.auth.user);

const invitationActionsMenu = (
const invitationActionsMenu = invitation && (
<Dropdown overlay={(
<Menu onClick={(action: MenuInfo) => {
if (action.key === MenuKeys.RESEND_INVITATION) {
Expand Down
12 changes: 10 additions & 2 deletions cvat/apps/iam/rules/memberships.rego
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ filter := [] { # Django Q object to filter list of entries
utils.is_admin
org_id := input.auth.organization.id
qobject := [ {"organization": org_id} ]
} else := qobject {
organizations.is_staff
org_id := input.auth.organization.id
qobject := [ {"organization": org_id} ]
} else := qobject {
org_id := input.auth.organization.id
qobject := [ {"organization": org_id}, {"is_active": true}, "&" ]
Expand All @@ -65,6 +69,12 @@ allow {
input.resource.user.id == input.auth.user.id
}

allow {
input.scope == utils.VIEW
organizations.is_staff
input.resource.organization.id == input.auth.organization.id
}

allow {
input.scope == utils.VIEW
input.resource.is_active
Expand All @@ -76,7 +86,6 @@ allow {
# himself/another maintainer/owner
allow {
{ utils.CHANGE_ROLE, utils.DELETE }[input.scope]
input.resource.is_active
input.resource.organization.id == input.auth.organization.id
utils.has_perm(utils.USER)
organizations.is_maintainer
Expand All @@ -91,7 +100,6 @@ allow {
# owner of the organization can change the role of any member and remove any member except himself
allow {
{ utils.CHANGE_ROLE, utils.DELETE }[input.scope]
input.resource.is_active
input.resource.organization.id == input.auth.organization.id
utils.has_perm(utils.USER)
organizations.is_owner
Expand Down
10 changes: 10 additions & 0 deletions cvat/apps/iam/rules/tests/generators/memberships_test.gen.rego.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,16 @@ def eval_rule(scope, context, ownership, privilege, membership, data):
return False

if scope != "create" and not data["resource"]["is_active"]:
is_staff = membership == "owner" or membership == 'maintainer'
if is_staff:
if scope != 'view':
if ORG_ROLES.index(membership) >= ORG_ROLES.index(resource["role"]):
return False
if GROUPS.index(privilege) > GROUPS.index("user"):
return False
if resource["user"]['id'] == data["auth"]["user"]['id']:
return False
return True
return False

return bool(rules)
Expand Down
11 changes: 9 additions & 2 deletions cvat/apps/organizations/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
# SPDX-License-Identifier: MIT

from django.contrib.auth import get_user_model
from allauth.account.models import EmailAddress
from allauth.account.adapter import get_adapter
from django.core.exceptions import ObjectDoesNotExist
from django.conf import settings
from django.contrib.auth.models import User
from django.utils.crypto import get_random_string
from django.db import transaction

from rest_framework import serializers
from distutils.util import strtobool
Expand Down Expand Up @@ -78,6 +81,7 @@ class Meta:
fields = ['key', 'created_date', 'owner', 'role', 'organization', 'email']
read_only_fields = ['key', 'created_date', 'owner', 'organization']

@transaction.atomic
def create(self, validated_data):
membership_data = validated_data.pop('membership')
organization = validated_data.pop('organization')
Expand All @@ -87,11 +91,12 @@ def create(self, validated_data):
del membership_data['user']
except ObjectDoesNotExist:
user_email = membership_data['user']['email']
username = user_email.split("@")[0]
user = User.objects.create_user(username=username, password=get_random_string(length=32),
user = User.objects.create_user(username=user_email, password=get_random_string(length=32),
email=user_email, is_active=False)
user.set_unusable_password()
email = EmailAddress.objects.create(user=user, email=user_email, primary=True, verified=False)
user.save()
email.save()
del membership_data['user']
membership, created = Membership.objects.get_or_create(
defaults=membership_data,
Expand Down Expand Up @@ -151,6 +156,8 @@ def save(self, request, invitation):
self.cleaned_data = self.get_cleaned_data()
user = invitation.membership.user
user.is_active = True
email = EmailAddress.objects.get(email=user.email)
get_adapter(request).confirm_email(request, email)
user.first_name = self.cleaned_data['first_name']
user.last_name = self.cleaned_data['last_name']
user.username = self.cleaned_data['username']
Expand Down

0 comments on commit 9819e6d

Please sign in to comment.