Skip to content

Commit c5c78bd

Browse files
authored
Agh gsr booking (#365)
* Implement AGH GSR booking (still need accurate gsr_data and database migration) * feat: update gsr_data with AGH gid, lid, rename LibCal credentials to GENERAL_LIBCAL_* for clarity * fix(gsr): fix AGH booking security vulnerabilities and improve reliability - Fix critical room authorization bypass in PennGroupsBookingWrapper - Replace insecure substring matching with precise room number extraction - Add HTTP status validation and consistent error messaging - Extract helper methods to eliminate code duplication - Map LibCal room names to PennGroups extensions correctly * Fix flake8 linting errors in gsr_booking Fixes whitespace, import ordering, line lengths, and other style issues to ensure code passes pre-commit hooks. * Fix remaining linting issues in gsr_booking - Add missing newline, fix import ordering, clean whitespace * Format gsr_booking with black and ruff - Split long lines and apply consistent formatting * feat: add SEAS booking support for Amy Gutmann Hall Add migration 0013 to support SEAS user bookings: - Add GroupMembership.is_seas field for SEAS affiliation tracking - Add PENNGRP to GSR.kind choices for SEAS locations - Update test expectations for AGH at position 2 in GSR data * Fix PennGroups API error handling in get_authorized_rooms Raise APIError on connection/API failures instead of returning None. Only return {} for SUBJECT_NOT_FOUND (not SEAS). Distinguishes API errors from user eligibility. * feat: adding gsr booking api credentials to base.py, modify add_pennlabs_users to include seas in zip * feat: Add SEAS status checking and improve API error handling - Integrate PennGroups API for SEAS membership verification - Add is_seas field and AGH room authorization checks - Standardize error handling: both is_wharton() and is_seas() now raise APIError - Add graceful fallbacks in model layer (check_wharton/check_seas) - Update tests with proper mocking for all API calls * feat: Implement GSR filtering by membership and fix test mocking - Add dynamic GSR filtering in Locations view based on Wharton/SEAS status - Penn Labs members see all GSRs (administrative override) - Fix test mock decorator conflicts by moving to method-level - Add comprehensive test coverage for location filtering (7 new tests) - Add mock JSON files for PennGroups API responses - Add graceful error handling for API failures All 67 GSR booking tests pass. Backward compatible. * fix: Use response data directly in location filtering tests Fix CI test failures by reading field from response data instead of looking up GSRs by ID. This makes tests more robust and environment-independent. All tests now use directly from the response instead of , avoiding DoesNotExist errors when GSR IDs differ between local and CI environments. * fix test_get_location_penn_labs_member and test github actions gsr loading * include gsr count in assert message to debug gsr counts in github actions * feat(gsr): improve PennGroups/AGH GSR booking tests - Fix deprecated assertEquals -> assertEqual - Add 7 new comprehensive tests for PennGroups/AGH functionality - Fix test_unauthorized_room_booking_fails mock - Add missing ConnectTimeout import * add clear cache to locations tests to hopefully fix ci/cd testing? * fix(gsr): implement per-permission-level caching for locations endpoint * fix: add back accidentally deleted task, slightly improve test documentation * Fix PennGroups API to use pennid and add local dev support - Update get_authorized_rooms to use pennid instead of username for PennGroups API calls - Add get_user_pennid helper to extract pennid from Platform API - Add bearer token fallback in get_user_info for local development when IPC fails - Update tests to mock get_user_pennid method - Add .env file with local development credentials * fix(gsr): handle LibCal space/item API returning list * fix: resolve merge conflicts with main in gsr_booking. * fix(gsr): correctly match both numeric and alphanumeric room identifiers. * refactor(gsr): simplify permission check error handling and add logging - Add logging to is_wharton() and is_seas() to track API failures - Return False instead of raising errors for failed permission checks - Remove need for try/except boilerplate from views, models, and commands
1 parent a4e6fa7 commit c5c78bd

23 files changed

Lines changed: 1864 additions & 86 deletions

backend/gsr_booking/api_wrapper.py

Lines changed: 406 additions & 24 deletions
Large diffs are not rendered by default.

backend/gsr_booking/data/gsr_data.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
LID,GID,NAME,SERVICE
22
JMHH,1,Huntsman,wharton
3+
20157,42437,Amy Gutmann Hall,penngroups
34
ARB,6,Academic Research,wharton
45
3620,6102,Albrecht Music Library,libcal
56
2634,1914,Penn Museum Library,libcal

backend/gsr_booking/management/commands/add_pennlabs_users.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from django.contrib.auth import get_user_model
22
from django.core.management.base import BaseCommand, CommandError
33

4-
from gsr_booking.api_wrapper import WhartonGSRBooker
4+
from gsr_booking.api_wrapper import PennGroupsGSRBooker, WhartonGSRBooker
55
from gsr_booking.models import Group, GroupMembership
66

77

@@ -21,6 +21,7 @@ def handle(self, *args, **options):
2121

2222
users = []
2323
wharton_statuses = []
24+
seas_statuses = []
2425

2526
input_count = int(input("How many users would you like to add? "))
2627
if input_count <= 0:
@@ -34,20 +35,30 @@ def handle(self, *args, **options):
3435
except User.DoesNotExist:
3536
self.stdout.write(f"User with PennKey {pennkey} does not exist. Skipping.")
3637
continue
38+
3739
users.append(user)
3840
is_wharton = WhartonGSRBooker.is_wharton(user)
3941
wharton_statuses.append(is_wharton)
42+
is_seas = PennGroupsGSRBooker.is_seas(user)
43+
seas_statuses.append(is_seas)
4044

4145
# confirm with the admin before proceeding
4246
self.stdout.write("The following users will be added to the Penn Labs group:")
43-
for user, is_wharton in zip(users, wharton_statuses):
44-
status = "Wharton" if is_wharton else "Regular"
47+
for user, is_wharton, is_seas in zip(users, wharton_statuses, seas_statuses):
48+
status_parts = []
49+
if is_wharton:
50+
status_parts.append("Wharton")
51+
if is_seas:
52+
status_parts.append("SEAS")
53+
if not status_parts:
54+
status_parts.append("Regular")
55+
status = " + ".join(status_parts)
4556
self.stdout.write(f"- {user.username} ({status})")
4657
confirm = input("Type 'yes' to confirm and proceed: ").strip().lower()
4758
if confirm != "yes":
4859
self.stdout.write("Aborted.")
4960
return
50-
for user, is_wharton in zip(users, wharton_statuses):
61+
for user, is_wharton, is_seas in zip(users, wharton_statuses, seas_statuses):
5162
membership, created = GroupMembership.objects.get_or_create(
5263
user=user,
5364
group=group,
@@ -56,6 +67,7 @@ def handle(self, *args, **options):
5667
"accepted": True,
5768
"pennkey_allow": True,
5869
"is_wharton": is_wharton,
70+
"is_seas": is_seas,
5971
},
6072
)
6173
if not created:
@@ -64,6 +76,7 @@ def handle(self, *args, **options):
6476
membership.accepted = True
6577
membership.pennkey_allow = True
6678
membership.is_wharton = is_wharton
79+
membership.is_seas = is_seas
6780
membership.save()
6881
self.stdout.write(f"Updated existing membership for {user.username}")
6982
else:

backend/gsr_booking/management/commands/load_gsrs.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
from django.core.management.base import BaseCommand
44

5-
from gsr_booking.models import GSR
5+
from gsr_booking.models import GSR, clear_gsr_location_caches
66

77

88
class Command(BaseCommand):
@@ -17,7 +17,17 @@ def handle(self, *args, **kwargs):
1717
image_url = (
1818
f"https://s3.us-east-2.amazonaws.com/labs.api/gsr/lid-{lid}-gid-{gid}.jpg"
1919
)
20-
kind = GSR.KIND_WHARTON if service == "wharton" else GSR.KIND_LIBCAL
21-
GSR.objects.create(lid=lid, gid=gid, name=name, kind=kind, image_url=image_url)
20+
kind = (
21+
GSR.KIND_PENNGROUPS
22+
if service == "penngroups"
23+
else GSR.KIND_WHARTON if service == "wharton" else GSR.KIND_LIBCAL
24+
)
25+
GSR.objects.update_or_create(
26+
lid=lid, gid=gid, defaults={"name": name, "kind": kind, "image_url": image_url}
27+
)
28+
29+
# Note: Caches are automatically cleared by post_save signals on GSR model
30+
# But we clear explicitly here as well for clarity and to handle edge cases
31+
clear_gsr_location_caches()
2232

23-
self.stdout.write("Uploaded GSRs!")
33+
self.stdout.write("Uploaded GSRs and cleared location caches!")
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
from django.contrib.auth import get_user_model
2+
from django.core.management.base import BaseCommand
3+
4+
from gsr_booking.api_wrapper import PennGroupsBookingWrapper
5+
from gsr_booking.models import GroupMembership
6+
7+
8+
class Command(BaseCommand):
9+
help = "Updates SEAS status for all users."
10+
11+
def handle(self, *args, **kwargs):
12+
users = GroupMembership.objects.values_list("user__username", flat=True).distinct()
13+
print(f"Checking {len(users)} users...")
14+
penngroups_wrapper = PennGroupsBookingWrapper()
15+
updated = 0
16+
17+
for username in users:
18+
user = get_user_model().objects.get(username=username)
19+
is_seas = penngroups_wrapper.is_seas(user)
20+
memberships = GroupMembership.objects.filter(user__username=user)
21+
for membership in memberships:
22+
if membership.is_seas != is_seas:
23+
membership.is_seas = is_seas
24+
membership.save()
25+
status = "now" if is_seas else "no longer"
26+
print(f"User {user} is {status} a SEAS user.")
27+
updated += 1
28+
29+
print(f"Done updating SEAS statuses. Updated: {updated} users.")

backend/gsr_booking/management/commands/update_wharton_status.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,24 @@
66

77

88
class Command(BaseCommand):
9-
help = "Updates Wharton privelige status for all users."
9+
help = "Updates Wharton privilege status for all users."
1010

1111
def handle(self, *args, **kwargs):
1212
users = GroupMembership.objects.values_list("user__username", flat=True).distinct()
1313
print(f"Checking {len(users)} users...")
14+
wharton_wrapper = WhartonBookingWrapper()
15+
updated = 0
16+
1417
for username in users:
1518
user = get_user_model().objects.get(username=username)
16-
is_wharton = WhartonBookingWrapper().is_wharton(user)
19+
is_wharton = wharton_wrapper.is_wharton(user)
1720
memberships = GroupMembership.objects.filter(user__username=user)
1821
for membership in memberships:
1922
if membership.is_wharton != is_wharton:
2023
membership.is_wharton = is_wharton
2124
membership.save()
2225
status = "now" if is_wharton else "no longer"
2326
print(f"User {user} is {status} a Wharton user.")
24-
print("Done updating Wharton statuses.")
27+
updated += 1
28+
29+
print(f"Done updating Wharton statuses. Updated: {updated} users.")
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Generated by Django 5.0.2 on 2025-10-26 18:09
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("gsr_booking", "0012_gsr_in_use"),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name="groupmembership",
15+
name="is_seas",
16+
field=models.BooleanField(blank=True, default=None, null=True),
17+
),
18+
migrations.AlterField(
19+
model_name="gsr",
20+
name="kind",
21+
field=models.CharField(
22+
choices=[("WHARTON", "Wharton"), ("LIBCAL", "Libcal"), ("PENNGRP", "Penngrp")],
23+
default="LIBCAL",
24+
max_length=7,
25+
),
26+
),
27+
]
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Generated by Django 5.0.2 on 2025-12-07 19:29
2+
3+
from django.db import migrations
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("gsr_booking", "0013_groupmembership_is_seas_alter_gsr_kind"),
10+
("gsr_booking", "0013_gsrsharecode"),
11+
]
12+
13+
operations = []

backend/gsr_booking/models.py

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import secrets
22

33
from django.contrib.auth import get_user_model
4+
from django.core.cache import cache
45
from django.db import models
6+
from django.db.models.signals import post_delete, post_save
7+
from django.dispatch import receiver
58
from django.utils import timezone
69

710

@@ -30,6 +33,8 @@ class GroupMembership(models.Model):
3033

3134
is_wharton = models.BooleanField(blank=True, null=True, default=None)
3235

36+
is_seas = models.BooleanField(blank=True, null=True, default=None)
37+
3338
@property
3439
def is_invite(self):
3540
return not self.accepted
@@ -38,14 +43,20 @@ def __str__(self):
3843
return f"{self.user}<->{self.group}"
3944

4045
def save(self, *args, **kwargs):
41-
# determines whether user is wharton or not
46+
# Determines whether user is wharton or not
4247
if self.is_wharton is None:
4348
self.is_wharton = self.check_wharton()
49+
# Determines whether user is seas or not
50+
if self.is_seas is None:
51+
self.is_seas = self.check_seas()
4452
super().save(*args, **kwargs)
4553

4654
def check_wharton(self):
4755
return WhartonGSRBooker.is_wharton(self.user)
4856

57+
def check_seas(self):
58+
return PennGroupsGSRBooker.is_seas(self.user)
59+
4960
class Meta:
5061
verbose_name = "Group Membership"
5162

@@ -95,7 +106,12 @@ class GSR(models.Model):
95106

96107
KIND_WHARTON = "WHARTON"
97108
KIND_LIBCAL = "LIBCAL"
98-
KIND_OPTIONS = ((KIND_WHARTON, "Wharton"), (KIND_LIBCAL, "Libcal"))
109+
KIND_PENNGROUPS = "PENNGRP"
110+
KIND_OPTIONS = (
111+
(KIND_WHARTON, "Wharton"),
112+
(KIND_LIBCAL, "Libcal"),
113+
(KIND_PENNGROUPS, "Penngrp"),
114+
)
99115

100116
kind = models.CharField(max_length=7, choices=KIND_OPTIONS, default=KIND_LIBCAL)
101117
lid = models.CharField(max_length=255)
@@ -163,4 +179,30 @@ def is_valid(self):
163179

164180

165181
# import at end to prevent circular dependency
166-
from gsr_booking.api_wrapper import WhartonGSRBooker # noqa: E402
182+
from gsr_booking.api_wrapper import PennGroupsGSRBooker, WhartonGSRBooker # noqa: E402
183+
184+
185+
# Signal handlers to clear location caches when GSR data changes
186+
def clear_gsr_location_caches():
187+
"""Clear all GSR location caches for all permission combinations"""
188+
cache_keys = [
189+
"gsr_locations:penn_labs",
190+
"gsr_locations:wharton_True_seas_True",
191+
"gsr_locations:wharton_True_seas_False",
192+
"gsr_locations:wharton_False_seas_True",
193+
"gsr_locations:wharton_False_seas_False",
194+
]
195+
for key in cache_keys:
196+
cache.delete(key)
197+
198+
199+
@receiver(post_save, sender=GSR)
200+
def clear_cache_on_gsr_save(sender, instance, **kwargs):
201+
"""Clear location caches when a GSR is saved (via admin or any other method)"""
202+
clear_gsr_location_caches()
203+
204+
205+
@receiver(post_delete, sender=GSR)
206+
def clear_cache_on_gsr_delete(sender, instance, **kwargs):
207+
"""Clear location caches when a GSR is deleted (via admin or any other method)"""
208+
clear_gsr_location_caches()

backend/gsr_booking/urls.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
from django.urls import include, path
2-
from django.views.decorators.cache import cache_page
32
from rest_framework import routers
43

54
from gsr_booking.views import (
65
Availability,
76
BookRoom,
87
CancelRoom,
8+
CheckSEAS,
99
CheckWharton,
1010
GroupMembershipViewSet,
1111
GroupViewSet,
@@ -15,7 +15,6 @@
1515
RecentGSRs,
1616
ReservationsView,
1717
)
18-
from utils.cache import Cache
1918

2019

2120
router = routers.DefaultRouter()
@@ -27,9 +26,10 @@
2726

2827
urlpatterns = [
2928
path("", include(router.urls)),
30-
path("locations/", cache_page(Cache.MONTH)(Locations.as_view()), name="locations"),
29+
path("locations/", Locations.as_view(), name="locations"),
3130
path("recent/", RecentGSRs.as_view(), name="recent-gsrs"),
3231
path("wharton/", CheckWharton.as_view(), name="is-wharton"),
32+
path("seas/", CheckSEAS.as_view(), name="is-seas"),
3333
path("availability/<lid>/<gid>", Availability.as_view(), name="availability"),
3434
path("book/", BookRoom.as_view(), name="book"),
3535
path("cancel/", CancelRoom.as_view(), name="cancel"),

0 commit comments

Comments
 (0)