Skip to content

Commit 786e840

Browse files
authored
B2B: Update Keycloak org membership when a user redeems an enrollment code (#2989)
1 parent 549cab3 commit 786e840

File tree

26 files changed

+981
-303
lines changed

26 files changed

+981
-303
lines changed

b2b/admin.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ class ContractPageAdmin(ReadOnlyModelAdmin):
119119
"title",
120120
"description",
121121
"integration_type",
122+
"membership_type",
122123
"contract_start",
123124
"contract_end",
124125
"max_learners",

b2b/api.py

Lines changed: 164 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,18 @@
1919

2020
from b2b.constants import (
2121
B2B_RUN_TAG_FORMAT,
22-
CONTRACT_INTEGRATION_SSO,
22+
CONTRACT_MEMBERSHIP_AUTOS,
2323
ORG_KEY_MAX_LENGTH,
2424
)
2525
from b2b.exceptions import SourceCourseIncompleteError, TargetCourseRunExistsError
2626
from b2b.keycloak_admin_api import KCAM_ORGANIZATIONS, get_keycloak_model
2727
from b2b.keycloak_admin_dataclasses import OrganizationRepresentation
28-
from b2b.models import ContractPage, OrganizationIndexPage, OrganizationPage
28+
from b2b.models import (
29+
ContractPage,
30+
OrganizationIndexPage,
31+
OrganizationPage,
32+
UserOrganization,
33+
)
2934
from cms.api import get_home_page
3035
from courses.constants import UAI_COURSEWARE_ID_PREFIX
3136
from courses.models import Course, CourseRun
@@ -596,7 +601,10 @@ def ensure_enrollment_codes_exist(contract: ContractPage):
596601
"""
597602
log.info("Checking enrollment codes for contract %s", contract)
598603

599-
if contract.integration_type == "sso" and not contract.enrollment_fixed_price:
604+
if (
605+
contract.integration_type in CONTRACT_MEMBERSHIP_AUTOS
606+
or contract.membership_type in CONTRACT_MEMBERSHIP_AUTOS
607+
) and not contract.enrollment_fixed_price:
600608
# SSO contracts w/out price don't need discounts.
601609
return _handle_sso_free_contract(contract)
602610

@@ -762,19 +770,30 @@ def create_b2b_enrollment(request, product: Product):
762770

763771
def reconcile_user_orgs(user, organizations):
764772
"""
765-
Reconcile the specified users with the provided organization list.
766-
767-
When we get a list of organizations from an authoritative source, we need to
768-
be able to parse that list and make sure the user's org attachments match.
769-
This will pull the contracts that the user belongs to that are also
770-
SSO-enabled, and will remove the user from the contract if they're not
771-
supposed to be in them. It will also add the user to any SSO-enabled contract
772-
that the org has.
773-
774-
This only considers contracts that are SSO-enabled and zero-cost. If the
775-
contract is seat limited, we will only add the user if there's room.
776-
(If there isn't, we will log an error.) Only SSO-enabled contracts are
777-
considered; any that the user is in that aren't SSO-enabled will be left alone.
773+
Reconcile the specified user with the provided organization list.
774+
775+
When we get a list of organizations from a source (so, in the user payload
776+
from APISIX) for a particular user, we need to ensure that the user's
777+
organization membership in MITx Online matches up with what we're given. In
778+
addition, once we've done that, we need to ensure they're also in the
779+
contracts that are marked as "managed". If the user is in an organization
780+
that isn't in the list we've received, we need to remove them; in addition,
781+
they should be removed from any "managed" contracts for the org they're in
782+
as well.
783+
784+
There is a special case where the user may be in an organization that isn't
785+
represented in the payload we're given. This happens when the user uses an
786+
enrollment code. We update the org membership here and in Keycloak, but the
787+
payload from APISIX won't include their updated org membership until their
788+
APISIX session expires. We have a flag on the many-to-many table that
789+
indicates that we should leave those memberships alone - otherwise, we'll
790+
inadvertently add them to the org and then immediately remove them. (Once
791+
the org _does_ show up in the list, we should clear the flag.)
792+
793+
We cache the user's org membership in redis to save some hits to the
794+
database. This gets hit on every authenticated request, so probably good to
795+
try to keep the query count low. The cache is a list of tuples of (org_uuid,
796+
not_expected_in_payload).
778797
779798
If the user is enrolled in any courses that are in a contract they'll be
780799
removed from, they will be left there. Not real sure what we should do in
@@ -791,61 +810,70 @@ def reconcile_user_orgs(user, organizations):
791810
user_org_cache_key = f"org-membership-cache-{user.id}"
792811
cached_org_membership = caches["redis"].get(user_org_cache_key, False)
793812

794-
if cached_org_membership and sorted(cached_org_membership) == sorted(organizations):
795-
log.info("reconcile_user_orgs: skipping reconcilation for %s", user.id)
796-
return (
797-
0,
798-
0,
799-
)
813+
if cached_org_membership:
814+
cached_expected_org_membership = [
815+
str(org_id)
816+
for org_id, not_expected_in_payload in cached_org_membership
817+
if not_expected_in_payload
818+
]
819+
820+
if sorted(cached_expected_org_membership) == sorted(organizations):
821+
log.info(
822+
"reconcile_user_orgs: everything OK, skipping reconcilation for %s",
823+
user.id,
824+
)
825+
return (
826+
0,
827+
0,
828+
)
800829

801830
log.info("reconcile_user_orgs: running reconcilation for %s", user.id)
802831

803-
user_contracts_qs = user.b2b_contracts.filter(
804-
integration_type=CONTRACT_INTEGRATION_SSO
805-
)
832+
# we've checked the cached org membership, so now figure out what orgs
833+
# we're in but aren't in the list, and vice versa
806834

807-
if len(organizations) == 0:
808-
# User has no orgs, so we should clear them from all SSO contracts.
809-
contracts_to_remove = user_contracts_qs.all()
810-
[user.b2b_contracts.remove(contract) for contract in contracts_to_remove]
811-
user.save()
812-
return (0, len(contracts_to_remove))
835+
orgs_to_add = OrganizationPage.objects.filter(
836+
Q(sso_organization_id__in=organizations) & ~Q(organization_users__user=user)
837+
).filter(sso_organization_id__isnull=False)
813838

814-
orgs = OrganizationPage.objects.filter(sso_organization_id__in=organizations).all()
815-
no_orgs = OrganizationPage.objects.exclude(
816-
sso_organization_id__in=organizations
817-
).all()
839+
orgs_to_remove = UserOrganization.objects.filter(
840+
~Q(organization__sso_organization_id__in=organizations)
841+
& Q(user=user, keep_until_seen=False)
842+
).filter(organization__sso_organization_id__isnull=False)
818843

819-
contracts_to_remove = user_contracts_qs.filter(organization__in=no_orgs).all()
844+
for add_org in orgs_to_add:
845+
# add org, add contracts, clear flag if we need to
846+
UserOrganization.objects.update_or_create(
847+
user=user,
848+
organization=add_org,
849+
defaults={"keep_until_seen": False},
850+
)
820851

821-
if contracts_to_remove.count() > 0:
822-
[
823-
user.b2b_contracts.remove(contract_to_remove)
824-
for contract_to_remove in contracts_to_remove
825-
]
852+
add_org.add_user_contracts(user)
853+
log.info("reconcile_user_orgs: added user %s to org %s", user.id, add_org)
826854

827-
contracts_to_add = (
828-
ContractPage.objects.filter(
829-
integration_type=CONTRACT_INTEGRATION_SSO, organization__in=orgs
855+
for remove_org in orgs_to_remove:
856+
# remove org, remove contracts
857+
remove_org.organization.remove_user_contracts(user)
858+
log.info(
859+
"reconcile_user_orgs: removed user %s from org %s", user.id, remove_org
830860
)
831-
.exclude(pk__in=user_contracts_qs.all().values_list("id", flat=True))
832-
.all()
833-
)
834-
835-
if contracts_to_add.count() > 0:
836-
[
837-
user.b2b_contracts.add(contract_to_add)
838-
for contract_to_add in contracts_to_add
839-
]
861+
remove_org.delete()
840862

841-
user.save()
842863
user.refresh_from_db()
843-
orgs = [str(org_id) for org_id in user.b2b_organization_sso_ids]
864+
orgs = [
865+
(str(org.organization.sso_organization_id), not org.keep_until_seen)
866+
for org in user.user_organizations.all()
867+
]
868+
869+
user.user_organizations.filter(
870+
organization__sso_organization_id__in=organizations, keep_until_seen=True
871+
).update(keep_until_seen=False)
844872

845873
user_org_cache_key = f"org-membership-cache-{user.id}"
846874
caches["redis"].set(user_org_cache_key, sorted(orgs))
847875

848-
return (len(contracts_to_add), len(contracts_to_remove))
876+
return (len(orgs_to_add), len(orgs_to_remove))
849877

850878

851879
def reconcile_single_keycloak_org(keycloak_org: OrganizationRepresentation):
@@ -925,3 +953,83 @@ def reconcile_keycloak_orgs():
925953
)
926954

927955
return (created_count, updated_count)
956+
957+
958+
def add_user_org_membership(org, user):
959+
"""
960+
Add a given user to a Keycloak organization.
961+
962+
If we're adding a user to a contract, and they're not in that contract's
963+
organization, we need to do that and update Keycloak as well. Since the user
964+
won't have the org in their user data list initially, we'll also need to
965+
flag the membership so we don't remove it immediately later in the
966+
middleware.
967+
968+
Args:
969+
- org (OrganizationPage): The organization to add the user to.
970+
- user (User): The user to add to the organization.
971+
Returns:
972+
- bool: True if the user was added, False otherwise.
973+
"""
974+
975+
org_model = get_keycloak_model(OrganizationRepresentation, "organizations")
976+
977+
kc_org = org_model.get(org.sso_organization_id)
978+
979+
if not kc_org:
980+
log.warning("No Keycloak organization found for %s", org.sso_organization_id)
981+
return False
982+
983+
return org_model.associate("members", org.sso_organization_id, user.global_id)
984+
985+
986+
def process_add_org_membership(user, organization, *, keep_until_seen=False):
987+
"""
988+
Add a user to an org, and kick off contract processing.
989+
990+
This allows us to manage UserOrganization records without necessarily
991+
being forced to process contract memberships at the same time.
992+
993+
Args:
994+
- user (users.models.User): the user to add
995+
- organization (b2b.models.OrganizationPage): the organization to add the user to
996+
- keep_until_seen (bool): if True, the user will be kept in the org until the
997+
organization is seen in their SSO data.
998+
"""
999+
1000+
obj, created = UserOrganization.objects.get_or_create(
1001+
user=user,
1002+
organization=organization,
1003+
)
1004+
if created:
1005+
obj.keep_until_seen = keep_until_seen
1006+
obj.save()
1007+
try:
1008+
organization.attach_user(user)
1009+
except ConnectionError:
1010+
log.exception(
1011+
"Could not attach %s to Keycloak org for %s", user, organization
1012+
)
1013+
organization.add_user_contracts(user)
1014+
1015+
return obj
1016+
1017+
1018+
def process_remove_org_membership(user, organization):
1019+
"""
1020+
Remove a user from an org, and kick off contract processing.
1021+
1022+
Other side of the process_add_org_membership function - removes the membership
1023+
and associated managed contracts.
1024+
1025+
Args:
1026+
- user (users.models.User): the user to remove
1027+
- organization (b2b.models.OrganizationPage): the organization to remove the user from
1028+
"""
1029+
1030+
organization.remove_user_contracts(user)
1031+
1032+
UserOrganization.objects.filter(
1033+
user=user,
1034+
organization=organization,
1035+
).get().delete()

0 commit comments

Comments
 (0)