Skip to content

Commit d393e80

Browse files
eguerranteguerrant
committed
save work, some UI and misc tests
1 parent 6e0f1e4 commit d393e80

File tree

4 files changed

+271
-7
lines changed

4 files changed

+271
-7
lines changed

‎api/operations/create_role_request.py‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313
OktaGroup,
1414
OktaGroupTagMap,
1515
OktaUser,
16+
OktaUserGroupMember,
1617
RoleGroup,
1718
RoleRequest,
19+
Tag
1820
)
1921
from api.models.app_group import get_access_owners, get_app_managers
2022
from api.models.okta_group import get_group_managers
23+
from api.models.tag import coalesce_constraints
2124
from api.operations.approve_role_request import ApproveRoleRequest
2225
from api.operations.reject_role_request import RejectRoleRequest
2326
from api.plugins import get_conditional_access_hook, get_notification_hook
@@ -46,6 +49,13 @@ def __init__(
4649
self.requester_role = (
4750
RoleGroup.query.filter(RoleGroup.deleted_at.is_(None)).filter(RoleGroup.id == requester_role).first()
4851
)
52+
# self.requester_role = (
53+
# db.session.query(RoleGroup)
54+
# .options(joinedload(OktaUserGroupMember.user))
55+
# .filter(RoleGroup.deleted_at.is_(None))
56+
# .filter(RoleGroup.id == requester_role)
57+
# .first()
58+
# )
4959
else:
5060
self.requester_role = requester_role
5161

@@ -90,6 +100,37 @@ def execute(self) -> Optional[RoleRequest]:
90100
# Fetch the users to notify
91101
approvers = get_group_managers(self.requested_group.id)
92102

103+
requested_group_tags = [tm.tag for tm in self.requested_group.active_group_tags]
104+
105+
role_memberships = [u.user_id for u in (
106+
OktaUserGroupMember.query.filter(OktaUserGroupMember.group_id == self.requester_role.id)
107+
.filter(OktaUserGroupMember.is_owner.is_(False))
108+
.filter(
109+
db.or_(
110+
OktaUserGroupMember.ended_at.is_(None),
111+
OktaUserGroupMember.ended_at > db.func.now(),
112+
)
113+
)
114+
.all()
115+
)]
116+
117+
# If group tagged with disallow self add constraint, filter out approvers who are also members of the role
118+
if self.request_ownership:
119+
disallow_self_add_owner = coalesce_constraints(
120+
constraint_key=Tag.DISALLOW_SELF_ADD_OWNERSHIP_CONSTRAINT_KEY,
121+
tags=requested_group_tags,
122+
)
123+
if disallow_self_add_owner:
124+
approvers = [a for a in approvers if a.id not in role_memberships]
125+
126+
else:
127+
disallow_self_add_member = coalesce_constraints(
128+
constraint_key=Tag.DISALLOW_SELF_ADD_MEMBERSHIP_CONSTRAINT_KEY,
129+
tags=requested_group_tags,
130+
)
131+
if disallow_self_add_member:
132+
approvers = [a for a in approvers if a.id not in role_memberships]
133+
93134
# If there are no approvers, try to get the app managers
94135
# or if the only approver is the requester, try to get the app managers
95136
if (

‎api/views/resources/role_request.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,9 @@ def get(self, role_request_id: str) -> ResponseReturnValue:
114114
@FlaskApiSpecDecorators.response_schema(RoleRequestSchema)
115115
def put(self, role_request_id: str) -> ResponseReturnValue:
116116
role_request = (
117-
RoleRequest.query.options(joinedload(RoleRequest.active_requested_group))
117+
RoleRequest.query.options(
118+
joinedload(RoleRequest.active_requested_group),
119+
joinedload(RoleRequest.requester_role))
118120
.filter(RoleRequest.id == role_request_id)
119121
.first_or_404()
120122
)

‎src/pages/role_requests/Read.tsx‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ export default function ReadRoleRequest() {
179179

180180
const roleRequest = data ?? ({} as RoleRequest);
181181

182-
console.log(roleRequest);
182+
// console.log(roleRequest);
183183

184184
const ownRequest = roleRequest.requester?.id == currentUser.id;
185185

@@ -196,6 +196,7 @@ export default function ReadRoleRequest() {
196196
? requestedUntilDelta.toString()
197197
: 'custom';
198198

199+
// TODO if owner can't add self constraint and owner member of role, set below to false and add note about constraint
199200
const requestedGroupManager = canManageGroup(currentUser, roleRequest.requested_group);
200201

201202
const complete = (
@@ -229,7 +230,7 @@ export default function ReadRoleRequest() {
229230

230231
const constraints = ComputeConstraints(roleRequest);
231232

232-
console.log(constraints);
233+
// console.log(constraints);
233234

234235
const timeLimit: number | null = constraints[0] as number | null;
235236
const reason: boolean = constraints[1] as boolean;
@@ -260,7 +261,9 @@ export default function ReadRoleRequest() {
260261
}));
261262
}
262263

263-
const ownerships = groupBy(group.active_user_ownerships, (m) => m.active_user?.id);
264+
// TODO if owner can't add self constraint, filter ownerships to remove roleRequest.requester_role?.active_user_memberships
265+
266+
let ownerships = groupBy(group.active_user_ownerships, (m) => m.active_user?.id);
264267

265268
const {data: appData} = useGetAppById(
266269
{
@@ -436,11 +439,11 @@ export default function ReadRoleRequest() {
436439
<Box component="span" sx={{color: 'primary.main', fontWeight: 'bold'}}>
437440
<> ownership </>
438441
</Box>{' '}
442+
of
439443
</>
440444
) : (
441-
' membership '
445+
' membership to '
442446
)}
443-
of
444447
</Typography>
445448
<Typography variant="h4">
446449
{(roleRequest.requested_group?.deleted_at ?? null) != null ? (

‎tests/test_role_request.py‎

Lines changed: 219 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from datetime import datetime, timedelta
1+
from datetime import datetime, timedelta, timezone
22
from typing import Any
33

44
from flask import Flask, url_for
@@ -641,6 +641,64 @@ def test_get_all_possible_role_request_approvers(app: Flask, mocker: MockerFixtu
641641
assert users[2] in approvers
642642

643643

644+
def test_role_request_approvers_tagged(
645+
app: Flask,
646+
db: SQLAlchemy,
647+
role_group: RoleGroup,
648+
okta_group: OktaGroup,
649+
user: OktaUser,
650+
tag: Tag,
651+
mocker: MockerFixture,
652+
) -> None:
653+
access_owner = OktaUser.query.filter(OktaUser.email == app.config["CURRENT_OKTA_USER_EMAIL"]).first()
654+
655+
user2 = OktaUserFactory.create()
656+
user3 = OktaUserFactory.create()
657+
db.session.add(user)
658+
db.session.add(user2)
659+
db.session.add(user3)
660+
db.session.add(okta_group)
661+
db.session.commit()
662+
663+
# Add role group that user owns
664+
db.session.add(role_group)
665+
db.session.commit()
666+
667+
ModifyGroupUsers(group=role_group, members_to_add=[user2.id], owners_to_add=[user.id], sync_to_okta=False).execute()
668+
ModifyGroupUsers(group=okta_group, owners_to_add=[user2.id, user3.id], sync_to_okta=False).execute()
669+
670+
# Add tag
671+
tag.constraints = {
672+
Tag.DISALLOW_SELF_ADD_MEMBERSHIP_CONSTRAINT_KEY: True,
673+
}
674+
db.session.add(tag)
675+
db.session.commit()
676+
677+
db.session.add(OktaGroupTagMap(group_id=okta_group.id, tag_id=tag.id))
678+
db.session.commit()
679+
680+
hook = get_notification_hook()
681+
request_created_notification_spy = mocker.patch.object(hook, "access_role_request_created")
682+
request_completed_notification_spy = mocker.patch.object(hook, "access_role_request_completed")
683+
684+
role_request = CreateRoleRequest(
685+
requester_user=user,
686+
requester_role=role_group,
687+
requested_group=okta_group,
688+
request_reason="test reason",
689+
).execute()
690+
691+
assert role_request is not None
692+
assert request_created_notification_spy.call_count == 1
693+
_, kwargs = request_created_notification_spy.call_args
694+
assert kwargs["role_request"] == role_request
695+
assert kwargs["role"] == role_group
696+
assert kwargs["group"] == okta_group
697+
assert kwargs["requester"] == user
698+
assert len(kwargs["approvers"]) == 1
699+
assert user3 in kwargs["approvers"]
700+
701+
644702
def test_resolve_app_role_request_notification(
645703
app: Flask,
646704
db: SQLAlchemy,
@@ -952,6 +1010,166 @@ def test_auto_resolve_create_role_request_with_time_limit_constraint_tag(
9521010
assert tag in kwargs["group_tags"]
9531011

9541012

1013+
def test_time_limit_constraint_tag(
1014+
app: Flask,
1015+
db: SQLAlchemy,
1016+
access_app: App,
1017+
role_group: RoleGroup,
1018+
app_group: AppGroup,
1019+
user: OktaUser,
1020+
tag: Tag,
1021+
mocker: MockerFixture,
1022+
) -> None:
1023+
access_owner = OktaUser.query.filter(OktaUser.email == app.config["CURRENT_OKTA_USER_EMAIL"]).first()
1024+
1025+
# Add User
1026+
db.session.add(user)
1027+
db.session.commit()
1028+
1029+
# Add app group that no one owns
1030+
app_group.app_id = access_app.id
1031+
app_group.is_owner = False
1032+
db.session.add(app_group)
1033+
1034+
db.session.commit()
1035+
1036+
# Add role group that user owns
1037+
db.session.add(role_group)
1038+
db.session.commit()
1039+
1040+
ModifyGroupUsers(group=role_group, members_to_add=[user.id], owners_to_add=[user.id], sync_to_okta=False).execute()
1041+
1042+
# Add tag
1043+
tag.constraints = {
1044+
Tag.MEMBER_TIME_LIMIT_CONSTRAINT_KEY: THREE_DAYS_IN_SECONDS,
1045+
}
1046+
db.session.add(tag)
1047+
db.session.commit()
1048+
1049+
db.session.add(OktaGroupTagMap(group_id=app_group.id, tag_id=tag.id))
1050+
db.session.commit()
1051+
1052+
hook = get_notification_hook()
1053+
request_created_notification_spy = mocker.patch.object(hook, "access_role_request_created")
1054+
1055+
# Make request for more than time limit
1056+
role_request = CreateRoleRequest(
1057+
requester_user=user,
1058+
requester_role=role_group,
1059+
requested_group=app_group,
1060+
request_reason="test reason",
1061+
).execute()
1062+
1063+
assert role_request is not None
1064+
assert request_created_notification_spy.call_count == 1
1065+
_, kwargs = request_created_notification_spy.call_args
1066+
assert kwargs["role_request"] == role_request
1067+
# Ending time is None (more than time limit)
1068+
assert kwargs["role_request"].request_ending_at == None
1069+
assert kwargs["role"] == role_group
1070+
assert kwargs["group"] == app_group
1071+
assert kwargs["requester"] == user
1072+
1073+
# Try to approve for more than time limit
1074+
ApproveRoleRequest(
1075+
role_request=role_request,
1076+
approver_user=access_owner,
1077+
ending_at=datetime.now()+timedelta(seconds=SEVEN_DAYS_IN_SECONDS)
1078+
).execute()
1079+
1080+
# Should only be approved for time limit if approved time is higher
1081+
approval = RoleGroupMap.query.filter(RoleGroupMap.role_group == role_group).first()
1082+
# Make sure approval time is correct (could be a couple milliseconds off from calculated which is okay)
1083+
approval_time = approval.ended_at.replace(tzinfo=timezone.utc)
1084+
expected_approval_time = datetime.now(timezone.utc) + timedelta(seconds=THREE_DAYS_IN_SECONDS)
1085+
assert expected_approval_time > approval_time
1086+
assert expected_approval_time - approval_time < timedelta(minutes=1)
1087+
1088+
1089+
def test_owner_cant_add_self_constraint_tag(
1090+
app: Flask,
1091+
db: SQLAlchemy,
1092+
access_app: App,
1093+
role_group: RoleGroup,
1094+
app_group: AppGroup,
1095+
user: OktaUser,
1096+
tag: Tag,
1097+
mocker: MockerFixture,
1098+
) -> None:
1099+
access_owner = OktaUser.query.filter(OktaUser.email == app.config["CURRENT_OKTA_USER_EMAIL"]).first()
1100+
1101+
# Add Users
1102+
user2 = OktaUserFactory.create()
1103+
db.session.add(user)
1104+
db.session.add(user2)
1105+
db.session.commit()
1106+
1107+
# Add app group that no one owns
1108+
app_group.app_id = access_app.id
1109+
app_group.is_owner = False
1110+
db.session.add(app_group)
1111+
1112+
db.session.commit()
1113+
1114+
# Add role group that user owns
1115+
db.session.add(role_group)
1116+
db.session.commit()
1117+
1118+
ModifyGroupUsers(group=role_group, members_to_add=[user.id, user2.id], owners_to_add=[user.id], sync_to_okta=False).execute()
1119+
ModifyGroupUsers(group=app_group, owners_to_add=[user2.id], sync_to_okta=False).execute()
1120+
1121+
# Add tag
1122+
tag.constraints = {
1123+
Tag.DISALLOW_SELF_ADD_MEMBERSHIP_CONSTRAINT_KEY: True,
1124+
}
1125+
db.session.add(tag)
1126+
db.session.commit()
1127+
1128+
db.session.add(OktaGroupTagMap(group_id=app_group.id, tag_id=tag.id))
1129+
db.session.commit()
1130+
1131+
hook = get_notification_hook()
1132+
request_created_notification_spy = mocker.patch.object(hook, "access_role_request_created")
1133+
request_completed_notification_spy = mocker.patch.object(hook, "access_role_request_completed")
1134+
add_membership_spy = mocker.patch.object(okta, "async_add_user_to_group")
1135+
1136+
role_request = CreateRoleRequest(
1137+
requester_user=user,
1138+
requester_role=role_group,
1139+
requested_group=app_group,
1140+
request_reason="test reason",
1141+
).execute()
1142+
1143+
assert role_request is not None
1144+
assert request_created_notification_spy.call_count == 1
1145+
_, kwargs = request_created_notification_spy.call_args
1146+
assert kwargs["role_request"] == role_request
1147+
assert kwargs["role"] == role_group
1148+
assert kwargs["group"] == app_group
1149+
assert kwargs["requester"] == user
1150+
1151+
# user2 owns the group and is a member of the requester role, should fail
1152+
should_fail = ApproveRoleRequest(
1153+
role_request=role_request,
1154+
approver_user=user2,
1155+
).execute()
1156+
assert should_fail.status == AccessRequestStatus.PENDING
1157+
1158+
should_pass = ApproveRoleRequest(
1159+
role_request=role_request,
1160+
approver_user=access_owner,
1161+
).execute()
1162+
assert should_pass.status == AccessRequestStatus.APPROVED
1163+
1164+
assert add_membership_spy.call_count == 2
1165+
assert request_completed_notification_spy.call_count == 1
1166+
_, kwargs = request_completed_notification_spy.call_args
1167+
assert kwargs["role_request"] == should_pass
1168+
assert kwargs["role"] == role_group
1169+
assert kwargs["group"] == app_group
1170+
assert kwargs["requester"] == user
1171+
1172+
9551173
def test_role_request_approval_via_direct_add(
9561174
client: FlaskClient,
9571175
app: Flask,

0 commit comments

Comments
 (0)