Bump ruff and fix type equalvalence checking issues (#93)

.github/workflows/lint.yml

@@ -23,7 +23,6 @@ jobs:
         python-version: ["3.12"]
     steps:
       - uses: actions/checkout@v4
-      - uses: chartboost/ruff-action@v1
       - name: Set up Python ${{ matrix.python-version }}
         uses: actions/setup-python@v5
         with:

api/models/access_request.py

@@ -15,7 +15,7 @@ def get_all_possible_request_approvers(access_request: AccessRequest) -> Set[Okt
 
     app_managers = []
 
-    if type(access_request.requested_group) == AppGroup:
+    if type(access_request.requested_group) is AppGroup:
         app_managers = get_app_managers(access_request.requested_group.app_id)
 
     return set(group_owners + access_app_owners + app_managers)

api/operations/constraints/check_for_reason.py

@@ -64,7 +64,7 @@ def execute_for_group(self) -> Tuple[bool, str]:
 
                 # If the group is a role group check to see if a reason is required for adding members or owners
                 # to the associated groups
-                if type(self.group) == RoleGroup and self.group.is_managed:
+                if type(self.group) is RoleGroup and self.group.is_managed:
                     member_groups = [rm.active_group for rm in self.group.active_role_associated_group_member_mappings]
                     for member_group in member_groups:
                         require_member_reason = coalesce_constraints(
@@ -92,7 +92,7 @@ def execute_for_group(self) -> Tuple[bool, str]:
         return True, ""
 
     def execute_for_role(self) -> Tuple[bool, str]:
-        if type(self.group) != RoleGroup:
+        if type(self.group) is not RoleGroup:
             return True, ""
 
         if self.invalid_reason(self.reason):

api/operations/constraints/check_for_self_add.py

@@ -79,7 +79,7 @@ def execute_for_group(self) -> Tuple[bool, str]:
 
             # If the group is a role group check to see if a reason is required for adding members or owners
             # to the associated groups
-            if type(self.group) == RoleGroup and self.group.is_managed:
+            if type(self.group) is RoleGroup and self.group.is_managed:
                 member_groups = [rm.active_group for rm in self.group.active_role_associated_group_member_mappings]
                 for member_group in member_groups:
                     disallow_self_add_membership = coalesce_constraints(
@@ -112,7 +112,7 @@ def execute_for_role(self) -> Tuple[bool, str]:
         if self.current_user is None or AuthorizationHelpers.is_access_admin(self.current_user.id):
             return True, ""
 
-        if type(self.group) != RoleGroup:
+        if type(self.group) is not RoleGroup:
             return True, ""
 
         # Check to see if the current user is a member of the role,

api/operations/create_access_request.py

@@ -82,9 +82,9 @@ def execute(self) -> Optional[AccessRequest]:
         # If there are no approvers, try to get the app managers
         # or if the only approver is the requester, try to get the app managers
         if (
-            (len(approvers) == 0 and type(self.requested_group) == AppGroup)
+            (len(approvers) == 0 and type(self.requested_group) is AppGroup)
             or (len(approvers) == 1 and approvers[0].id == self.requester.id)
-            and type(self.requested_group) == AppGroup
+            and type(self.requested_group) is AppGroup
         ):
             approvers = get_app_managers(self.requested_group.app_id)
 

api/operations/create_app.py

@@ -129,7 +129,7 @@ def execute(self) -> App:
             owner_app_group = CreateGroup(group=owner_app_group, current_user_id=self.current_user_id).execute()
         else:
             group_id = existing_owner_group.id
-            if type(existing_owner_group) != AppGroup:
+            if type(existing_owner_group) is not AppGroup:
                 ModifyGroupType(
                     group=existing_owner_group,
                     group_changes=AppGroup(app_id=app_id, is_owner=True),
@@ -168,7 +168,7 @@ def execute(self) -> App:
         )
         existing_app_group_ids_to_update = []
         for existing_app_group in other_existing_app_groups:
-            if type(existing_app_group) == AppGroup:
+            if type(existing_app_group) is AppGroup:
                 existing_app_group.app_id = app_id
                 existing_app_group.is_owner = False
             else:
@@ -198,7 +198,7 @@ def execute(self) -> App:
                     CreateGroup(group=app_group, current_user_id=self.current_user_id).execute()
                 else:
                     group_id = existing_group.id
-                    if type(existing_owner_group) != AppGroup:
+                    if type(existing_owner_group) is not AppGroup:
                         ModifyGroupType(
                             group=existing_owner_group,
                             group_changes=AppGroup(app_id=app_id, is_owner=False),

api/operations/create_group.py

@@ -45,7 +45,7 @@ def execute(self, *, _group: Optional[T] = None) -> T:
 
         # Make sure the app exists if we're creating an app group
         if (
-            type(self.group) == AppGroup
+            type(self.group) is AppGroup
             and App.query.filter(App.id == self.group.app_id).filter(App.deleted_at.is_(None)).first() is None
         ):
             raise ValueError("App for AppGroup does not exist")
@@ -58,7 +58,7 @@ def execute(self, *, _group: Optional[T] = None) -> T:
         db.session.commit()
 
         # If this is an app group, add any app tags
-        if type(self.group) == AppGroup:
+        if type(self.group) is AppGroup:
             app_tag_maps = (
                 AppTagMap.query.filter(AppTagMap.app_id == self.group.app_id)
                 .filter(

api/operations/delete_group.py

@@ -48,7 +48,7 @@ async def _execute(self) -> None:
         okta_tasks = []
 
         # Prevent deletion of the Access owner group
-        if type(self.group) == AppGroup and self.group.is_owner:
+        if type(self.group) is AppGroup and self.group.is_owner:
             app = App.query.filter(App.id == self.group.app_id).filter(App.deleted_at.is_(None)).first()
             if app is not None and app.name == App.ACCESS_APP_RESERVED_NAME:
                 raise ValueError("Access application owner group cannot be deleted")
@@ -110,7 +110,7 @@ async def _execute(self) -> None:
             {RoleGroupMap.ended_at: db.func.now()}, synchronize_session="fetch"
         )
 
-        if type(self.group) == RoleGroup:
+        if type(self.group) is RoleGroup:
             # End all group memberships via the role grant
             OktaUserGroupMember.query.filter(
                 db.or_(

api/operations/modify_group_type.py

@@ -40,13 +40,12 @@ def __init__(self, *, group: OktaGroup | str, group_changes: OktaGroup, current_
 
     def execute(self) -> OktaGroup:
         # Update group type if it's being modified
-        # Ignore Ruff because we don't want to include subclasses with an isinstance comparison
-        if type(self.group) != type(self.group_changes):  # noqa: E721
+        if type(self.group) is not type(self.group_changes):
             group_id = self.group.id
             old_group_type = self.group.type
 
             # Clean-up the old child table row
-            if type(self.group) == RoleGroup:
+            if type(self.group) is RoleGroup:
                 # End all group attachments to this role and all group memberships via the role grant
                 active_role_associated_groups = RoleGroupMap.query.filter(
                     db.or_(
@@ -63,7 +62,7 @@ def execute(self) -> OktaGroup:
                 db.session.commit()
 
                 db.session.execute(delete(RoleGroup.__table__).where(RoleGroup.__table__.c.id == group_id))
-            elif type(self.group) == AppGroup:
+            elif type(self.group) is AppGroup:
                 # Bail if this is the owner group for the app
                 # which cannot have its type changed
                 if self.group.is_owner:
@@ -96,7 +95,7 @@ def execute(self) -> OktaGroup:
             self.group = OktaGroup.query.filter(OktaGroup.deleted_at.is_(None)).filter(OktaGroup.id == group_id).first()
 
             # Create new child table row
-            if type(self.group_changes) == RoleGroup:
+            if type(self.group_changes) is RoleGroup:
                 # Convert any group memberships and ownerships via a role to direct group memberships and ownerships
                 active_group_users_from_role = (
                     OktaUserGroupMember.query.filter(
@@ -153,7 +152,7 @@ def execute(self) -> OktaGroup:
                         ).execute()
 
                 db.session.execute(insert(RoleGroup.__table__).values(id=group_id))
-            elif type(self.group_changes) == AppGroup:
+            elif type(self.group_changes) is AppGroup:
                 db.session.execute(
                     insert(AppGroup.__table__).values(
                         id=group_id,
@@ -170,7 +169,7 @@ def execute(self) -> OktaGroup:
             db.session.expunge_all()
 
             # Add all app tags to this new app group, after we've updated the group type
-            if type(self.group_changes) == AppGroup:
+            if type(self.group_changes) is AppGroup:
                 app_tag_maps = (
                     AppTagMap.query.options(joinedload(AppTagMap.active_tag))
                     .filter(

api/operations/modify_group_users.py

@@ -206,7 +206,7 @@ async def _execute(self) -> OktaGroup:
             )
 
             # For role groups, members to be removed should also be removed from all role associated groups
-            if type(self.group) == RoleGroup:
+            if type(self.group) is RoleGroup:
                 role_associated_groups_mappings = (
                     RoleGroupMap.query.filter(
                         db.or_(
@@ -274,7 +274,7 @@ async def _execute(self) -> OktaGroup:
                     async_tasks.append(asyncio.create_task(okta.async_remove_owner_from_group(self.group.id, owner_id)))
 
             # For role groups, members to be removed should also be removed from all Access-managed role associated groups
-            if type(self.group) == RoleGroup and self.sync_to_okta:
+            if type(self.group) is RoleGroup and self.sync_to_okta:
                 role_associated_groups_mappings = (
                     RoleGroupMap.query.options(joinedload(RoleGroupMap.active_group))
                     .join(RoleGroupMap.active_group)
@@ -388,7 +388,7 @@ async def _execute(self) -> OktaGroup:
                 db.session.add(ownership_to_add)
 
             # For role groups, new members should also be added to all Access-managed role associated groups
-            if type(self.group) == RoleGroup:
+            if type(self.group) is RoleGroup:
                 role_associated_groups_mappings = (
                     RoleGroupMap.query.options(joinedload(RoleGroupMap.active_group))
                     .join(RoleGroupMap.active_group)

api/syncer.py

@@ -526,7 +526,7 @@ def expiring_access_notifications_owner() -> None:
         owners = get_group_managers(group.id)
 
         if len(owners) == 0:
-            owners += get_app_managers(group.app_id) if type(group) == AppGroup else []
+            owners += get_app_managers(group.app_id) if type(group) is AppGroup else []
 
         if len(owners) == 0:
             owners = access_owners
@@ -563,7 +563,7 @@ def expiring_access_notifications_owner() -> None:
         owners = get_group_managers(group.id)
 
         if len(owners) == 0:
-            owners += get_app_managers(group.app_id) if type(group) == AppGroup else []
+            owners += get_app_managers(group.app_id) if type(group) is AppGroup else []
 
         if len(owners) == 0:
             owners = access_owners
@@ -632,7 +632,7 @@ def expiring_access_notifications_owner() -> None:
         owners = get_group_managers(group.id)
 
         if len(owners) == 0:
-            owners += get_app_managers(group.app_id) if type(group) == AppGroup else []
+            owners += get_app_managers(group.app_id) if type(group) is AppGroup else []
 
         if len(owners) == 0:
             owners = access_owners
@@ -666,7 +666,7 @@ def expiring_access_notifications_owner() -> None:
         owners = get_group_managers(group.id)
 
         if len(owners) == 0:
-            owners += get_app_managers(group.app_id) if type(group) == AppGroup else []
+            owners += get_app_managers(group.app_id) if type(group) is AppGroup else []
 
         if len(owners) == 0:
             owners = access_owners

api/views/resources/group.py

@@ -126,7 +126,7 @@ def put(self, group: OktaGroup) -> ResponseReturnValue:
                 )
 
         # Do not allow non-tag modifications of app owner groups (including Access app owner group)
-        if type(group) == AppGroup and group.is_owner:
+        if type(group) is AppGroup and group.is_owner:
             if len(json_data.get("tags_to_add", [])) > 0 or len(json_data.get("tags_to_remove", [])) > 0:
                 ModifyGroupTags(
                     group=group,
@@ -160,7 +160,7 @@ def put(self, group: OktaGroup) -> ResponseReturnValue:
                 abort(400, "Group already exists with the same name")
 
         # Update group type if it's being modified
-        if type(group) != type(group_changes):
+        if type(group) is not type(group_changes):
             # Only access admins should be able to change group types
             if not AuthorizationHelpers.is_access_admin():
                 abort(
@@ -221,7 +221,7 @@ def delete(self, group: OktaGroup) -> ResponseReturnValue:
                 "Groups not managed by Access cannot be modified",
             )
         # Do not allow deletion of app owner groups (including the Access app owner group)
-        if type(group) == AppGroup and group.is_owner:
+        if type(group) is AppGroup and group.is_owner:
             abort(
                 400,
                 "Application owner groups cannot be deleted without first deleting the application",

api/views/resources/webhook.py

@@ -100,7 +100,7 @@ def post(self) -> ResponseReturnValue:
                 ).execute()
 
             # If the user has access to this group via a role, remove them from the role
-            if type(group) != RoleGroup and event.get("eventType") == "group.user_membership.remove":
+            if type(group) is not RoleGroup and event.get("eventType") == "group.user_membership.remove":
                 active_role_user_group_memberships = (
                     OktaUserGroupMember.query.options(
                         joinedload(OktaUserGroupMember.active_role_group_mapping).joinedload(

requirements.txt

@@ -43,7 +43,7 @@ pluggy==1.5.0
 # Test - TODO: Move test packages to separate file
 tox==4.15.1
 # Lint
-ruff==0.4.8
+ruff==0.5.0
 # Typing
 mypy==1.10.0
 types-google-cloud-ndb==2.3.0.20240311

tox.ini

@@ -40,7 +40,7 @@ setenv =
 
 [testenv:ruff]
 commands =
-  ruff check --diff .
+  ruff check .
   ruff format --check --diff .
 
 [testenv:mypy]