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

AD interaction improvements and UI tweaks #295

Merged
merged 3 commits into from
Jun 17, 2024
Merged

AD interaction improvements and UI tweaks #295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e619e0f
add more checks
claire-peters Jun 13, 2024
6417108
restrict add/remove user settings, tweak UI
claire-peters Jun 17, 2024
355bba4
add raise_error
claire-peters Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coldfront/config/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
ALLOCATION_DEFAULT_ALLOCATION_LENGTH = ENV.int('ALLOCATION_DEFAULT_ALLOCATION_LENGTH', default=365)

# Categorization of allocation statuses
PENDING_ALLOCATION_STATUSES = ['New', 'In Progress', 'On Hold']
PENDING_ALLOCATION_STATUSES = ['New', 'In Progress', 'On Hold', 'Pending Activation']
ACTIVE_ALLOCATION_STATUSES = ['Active']
PENDING_ACTIVE_ALLOCATION_STATUSES = PENDING_ALLOCATION_STATUSES + ACTIVE_ALLOCATION_STATUSES
INACTIVE_ALLOCATION_STATUSES = ['Denied', 'Expired', 'Inactive', 'Pending Deactivation']
Expand Down
4 changes: 2 additions & 2 deletions coldfront/core/allocation/templates/allocation/allocation_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ <h3><i class="fas fa-list" aria-hidden="true"></i> Allocation Information</h3>
<td>
{% if allocation.get_resources_as_list %}
{% for resource in allocation.get_resources_as_list %}
{% if request.user.is_superuser and allocation.status.name == "New" and "Tier" in allocation.get_resources_as_string %}
{% if request.user.is_superuser and allocation.status.name in "New,In Progress,On Hold" and "Tier" in allocation.get_resources_as_string %}
{{ form.resource }} <a color="red">Please choose a resource.</a>
{% else %}
<a href="{% url 'resource-detail' resource.pk %}">{{ resource }}</a>
Expand Down Expand Up @@ -296,7 +296,7 @@ <h3><i class="fas fa-list" aria-hidden="true"></i> Allocation Information</h3>
</div>

<!-- New Allocation Action Form -->
{% if request.user.is_superuser or request.user.is_staff and allocation.status.name == 'New' %}
{% if request.user.is_superuser or request.user.is_staff and allocation.status.name in 'New,In Progress,On Hold' %}
<div class="container mb-3 mt-3">
<p class="text-justify"><i>
To approve the request, select a resource volume in the field above and fill out the form below before clicking "Approve". To deny the request, simply click "Deny".
Expand Down
16 changes: 11 additions & 5 deletions coldfront/core/project/templates/project/project_detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,20 +234,24 @@ <h3 class="d-inline"><i class="fas fa-list" aria-hidden="true"></i> Allocation H
<tbody>
{% for allocation, record in allocation_history_records %}
<tr style="background-color:#FFFFFF">
<td>{{ allocation.get_parent_resource.name }}</td>
<td>
<a href="{% url 'allocation-detail' allocation.pk %}">
{{ allocation.get_parent_resource.name }}
</a>
</td>
<td>{{ allocation.get_parent_resource.resource_type.name }}</td>
<td>{{ allocation.path }}</td>
<td>{{ allocation.allocationuser_set.count }}</td>
<td>{{ allocation.size|floatformat:1 }}</td>

{% if allocation.status.name in 'New,In Progress,On Hold' %}
<td class="text-info">{{ Requested }}</td>
<td class="text-success">Requested</td>
{% elif allocation.status.name == 'Active' %}
<td class="text-success">{{ allocation.status.name }}</td>
{% elif allocation.status.name in 'Pending Deactivation,Denied,Inactive' %}
<td class="text-danger">{{ allocation.status.name }}</td>
{% else %}
<td class="text-info">{{ allocation.status.name }}</td>
<td class="text-success">{{ allocation.status.name }}</td>
{% endif %}
<td>
{% if allocation.status.name not in 'New,Denied' %}
Expand Down Expand Up @@ -297,8 +301,10 @@ <h3 class="d-inline" id="users"><i class="fas fa-users" aria-hidden="true"></i>
<div class="float-right">
{% if project.status.name != 'Archived' and is_allowed_to_update_project %}
<a class="btn btn-primary" href="{{mailto}}" role="button"><i class="far fa-envelope" aria-hidden="true"></i> Email Project Users</a>
<a class="btn btn-success" href="{% url 'project-add-users-search' project.id %}" role="button"><i class="fas fa-user-plus" aria-hidden="true"></i> Add Users</a>
<a class="btn btn-danger" href="{% url 'project-remove-users' project.id %}" role="button"><i class="fas fa-user-times" aria-hidden="true"></i> Remove Users</a>
{% if request.user.is_superuser or request.user == project.pi %}
<a class="btn btn-success" href="{% url 'project-add-users-search' project.id %}" role="button"><i class="fas fa-user-plus" aria-hidden="true"></i> Add Users</a>
<a class="btn btn-danger" href="{% url 'project-remove-users' project.id %}" role="button"><i class="fas fa-user-times" aria-hidden="true"></i> Remove Users</a>
{% endif %}
{% endif %}
</div>
</div>
Expand Down
24 changes: 17 additions & 7 deletions coldfront/core/project/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,8 @@ class ProjectAddUsersSearchView(LoginRequiredMixin, UserPassesTestMixin, Templat
def test_func(self):
"""UserPassesTestMixin Tests"""
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
# if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
if self.request.user.is_superuser or self.request.user == project.pi:
return True
return False

Expand Down Expand Up @@ -488,7 +489,8 @@ class ProjectAddUsersSearchResultsView(
def test_func(self):
"""UserPassesTestMixin Tests"""
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
# if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
if self.request.user.is_superuser or self.request.user == project.pi:
return True
return False

Expand Down Expand Up @@ -560,7 +562,8 @@ class ProjectAddUsersView(LoginRequiredMixin, UserPassesTestMixin, View):
def test_func(self):
"""UserPassesTestMixin Tests"""
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
# if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
if self.request.user.is_superuser or self.request.user == project.pi:
return True
err = 'You do not have permission to add users to the project.'
messages.error(self.request, err)
Expand Down Expand Up @@ -616,6 +619,8 @@ def post(self, request, *args, **kwargs):
allocation_form_data = allocation_form.cleaned_data['allocation']
if '__select_all__' in allocation_form_data:
allocation_form_data.remove('__select_all__')
errors = []
successes = []
for form in formset:
user_form_data = form.cleaned_data
if user_form_data['selected']:
Expand All @@ -636,7 +641,7 @@ def post(self, request, *args, **kwargs):
except Exception as e:
error = f"Could not locate user for {user_username}: {e}"
logger.error('P665: %s', error)
messages.error(request, error)
errors.append(error)
continue

role_choice = user_form_data.get('role')
Expand All @@ -653,13 +658,14 @@ def post(self, request, *args, **kwargs):
project_obj.title,
)
except Exception as e:
error = f"Could not add user {user_obj} to AD Group for {project_obj.title}: {e}"
error = f"Could not add user {user_obj} to AD Group for {project_obj.title}: {e}\nPlease contact Coldfront administration for further assistance."
logger.error(
"P685: user %s could not add AD user of %s to AD Group of %s: %s",
self.request.user, user_obj, project_obj.title, e
)
messages.error(request, error)
errors.append(error)
continue
successes.append(f"User {user_obj} added to AD Group for {project_obj.title}")

# Is the user already in the project?
project_obj.projectuser_set.update_or_create(
Expand Down Expand Up @@ -690,6 +696,9 @@ def post(self, request, *args, **kwargs):
sender=self.__class__,
allocation_user_pk=allocation_user_obj.pk,
)
if errors:
for error in errors:
messages.error(request, error)

messages.success(request, f'Added {added_users_count} users to project.')
else:
Expand All @@ -710,7 +719,8 @@ class ProjectRemoveUsersView(LoginRequiredMixin, UserPassesTestMixin, TemplateVi
def test_func(self):
"""UserPassesTestMixin Tests"""
project_obj = get_object_or_404(Project, pk=self.kwargs.get('pk'))
if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
# if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE):
if self.request.user.is_superuser or self.request.user == project.pi:
return True
return False

Expand Down
34 changes: 28 additions & 6 deletions coldfront/plugins/ldap/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@

username_ignore_list = import_from_settings('username_ignore_list', [])

class LDAPException(Exception):
"""The base exception class for all LDAPExceptions"""

class LDAPUserAdditionError(LDAPException):
"""An exception raised when a user cannot be added to an LDAP Group"""

class LDAPUserRemovalError(LDAPException):
"""An exception raised when a user cannot be removed from an LDAP Group"""

class LDAPConn:
"""LDAP connection object
"""
Expand Down Expand Up @@ -71,6 +80,11 @@ def __init__(self, test=False):
self.server, self.LDAP_BIND_DN, self.LDAP_BIND_PASSWORD, auto_bind=True
)

def member_in_group(self, member_dn, group_dn):
"""Check if a member is in a group"""
group = self.search_groups({'distinguishedName': group_dn, 'member': member_dn})
return bool(group)

def search(self, attr_search_dict, search_base, attributes=ALL_ATTRIBUTES):
"""Run an LDAP search.

Expand Down Expand Up @@ -176,13 +190,16 @@ def add_group_to_group(self, group_name, parent_group_name):
self.add_member_to_group(group, parent_group)

def add_member_to_group(self, member, group):
group_dn = group['distinguishedName']
member_dn = member['distinguishedName']
group_dn = group['distinguishedName']
try:
result = ad_add_members_to_groups(
self.conn, [member_dn], group_dn, fix=True)
self.conn, [member_dn], group_dn, fix=True, raise_error=True)
except Exception as e:
raise e
logger.exception("Error encountered while adding user to group: %s", e)
raise LDAPUserAdditionError("Error adding user to group.")
if not self.member_in_group(member_dn, group_dn):
raise LDAPUserAdditionError("Member not successfully added to group.")
return result

def remove_member_from_group(self, user_name, group_name):
Expand All @@ -192,13 +209,18 @@ def remove_member_from_group(self, user_name, group_name):
user = self.return_user_by_name(user_name)
if user['gidNumber'] == group['gidNumber']:
raise ValueError(
"Group is user's primary group. Please contact FASRC support to remove this user from your group.")
"Group is member's primary group. Please contact FASRC support to remove this member from your group.")
group_dn = group['distinguishedName']
user_dn = user['distinguishedName']
try:
result = ad_remove_members_from_groups(self.conn, [user_dn], group_dn, fix=True)
result = ad_remove_members_from_groups(
self.conn, [user_dn], group_dn, fix=True, raise_error=True
)
except Exception as e:
raise e
logger.exception("Error encountered while removing user from group: %s", e)
raise LDAPUserRemovalError("Error removing user from group.")
if self.member_in_group(user_dn, group_dn):
raise LDAPUserRemovalError("Member not successfully removed from group.")
return result

def users_in_primary_group(self, usernames, groupname):
Expand Down
Loading