[MST-1473] Proctoring Service Launch Implementation #275

MichaelRoytman · 2022-08-15T20:19:29Z

Description

This pull request implements the LTI Proctoring Services standard LTI messages and proctoring flow. It does not implement the Assessment Control Service.

This pull request relies on the location being fully decoupled from the XBlock (#273) and on the ability for consuming contexts to pass relevant context to the the library. As such, this is a work-in-progress and will evolve as those requirements are met.

Because of the dependency on the XBlock, you will find a number of TODOs in this pull request. I am hoping to address these iteratively as I receive feedback.

I have also not added tests yet, to avoid churn.

Key Details

I added a set of views that implement the LTI Proctoring Services launch flow: start_proctoring, launch_gate_endpoint_proctoring, start_assessment and end_assessment. You'll note that launch_gate_endpoint_proctoring has a lot of overlap with launch_gate_endpoint in [BB-5559] Decouple LTI 1.3 from LTI Consumer XBlock functionality #254. Because of the dependency on the XBlock, I could not easily reuse the logic there; I felt having a separate view for now would be easier.
I added an LtiProctoringConsumer.
I added the above views to the URL configuration.

Dependencies

[BB-5559] Decouple LTI 1.3 from LTI Consumer XBlock functionality #254

Testing Instructions

Setup Steps:

Clone the xblock-lti-consumer repository into your {workspace}/src directory. {workspace} should be the directory in which you have your other edX git repositories and should be the same directory your devstack repository is located in.
Check out this branch (mroytman/MST-1473-proctoring-service-launch) locally.
Clone the edx-exams repository into your {workspace} directory. {workspace} should be the directory in which you have your other edX git repositories and should be the same directory your devstack repository is located in.
Check out the branch mroytman/MST-1473-proctoring-service-launch locally.
Follow the One Time Setup instructions.

Testing Instructions:

If you want to test this implementation against the IMS Proctoring Specification testing tool, follow these steps.

Sign up for an IMS account here, if you are not already a member. The approval process should take around a day.
You will need to set up port forwarding so that your running Django server can connect with the IMS testing tool. You can use ngrok for this or any other tool you like. ngrok has comprehensive documentation for how to accomplish this.
In your local checkout of mroytman/MST-1473-proctoring-service-launch in edx-exams, change LMS_ROOT_URL in local.py to the forwarded URL. For example, https://9c11-73-238-153-173.ngrok.io.
Once your membership has been approved, go to the testing tool here.
In the testing tool, click Get Started.
Enter your name, email, and organization and click Confirm.
Click Create under Create New Product.
Enter a Product and Version and click Confirm. These values can be anything for the purposes of testing.
Click Select under Assessment Platform.
Click Begin Configuration.
Open up the Django admin by visiting localhost:18740/admin and logging in with the default edX development login credentials.
Under LTI_CONSUMER > Lti configuration, click Add.
Enter the following configuration:
15.1. Version: LTI 1.3 (with LTI Advantage Support)
13.2. Config store: Configuration Stored on this model.
From the testing tool, copy the following configuration into the Lti configuration model in the Django admin.
14.1. Tool End-Point to Receive Launches > Lti 1p3 launch url
14.2. OIDC Login Initiation URL > Lti 1p3 oidc url
Click Next.
Enter the following configuration in the testing tool. In this example, {root} refers to the URL that is being forwarded. For example, https://9c11-73-238-153-173.ngrok.io.
16.1. Testing iss Value: {root}
16.2. OIDC Auth URL: {root}/lti/launch_proctoring
16.3. Platform Well-Known/JWKS URL: {root}/lti/public_keyset
16.4. Client Id (Assigned to Cert Suite) > Enter the Lti 1p3 client id from the Lti configuration model in the Django admin.
Click Next.
Enter 1 for the deployment ID and click Next.
From the testing tool, copy theProctoring Tool JWKS URL into the Lti configuration model in the Django admin in the Lti 1p3 tool keyset url field.
Click Next.
Check Pause on OIDC Exchanges to Inspect Values and/or Pause Prior to JWT Messages to Inspect Values depending on whether you want interstitials before Tool requests or responses to the Assessment Platform.
Visit {root}/api/start_proctoring to kick off the flow.

MichaelRoytman · 2022-08-15T21:59:42Z

lti_consumer/plugin/views.py

+
+    # These claims are optional. They are necessary to set in order to properly verify the verified_user claim,
+    # if the Proctoring Tool includes it in the JWT.
+    # TODO: This will need to have additional consideration for PII.


Do we need to have a discussion with legal about exposing PII through the optional user identity claims? Can I safely send over the claims defined in docs/decisions/0005-lti-pii-sharing-flag.rst?

I don't think there's an issue here because we aren't allowing course teams to configure their own providers. Any available vendor will have a privacy agreement with us.

MichaelRoytman · 2022-08-15T22:03:07Z

lti_consumer/plugin/views.py

+# See 3.1.2.1.Authentication Request of the OIDC specification.
+# https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
+@require_http_methods(["GET", "POST"])
+def launch_gate_endpoint_proctoring(request, suffix=None):  # pylint: disable=unused-argument


A big question is how consumers of the xblock-lti-consumer library can pass relevant context to the library, including the context claim. For proctoring, this also includes data like attempt_number, resource_link, and start_assessment_url.

For the sake of getting this working, I have included the proctoring specific context as query parameters or form paramaters. I'm open to feedback.

MichaelRoytman · 2022-08-15T22:08:54Z

lti_consumer/plugin/views.py

@@ -501,3 +512,398 @@ def list(self, *args, **kwargs):
                "error": "above_response_limit",
                "explanation": "The number of retrieved users is bigger than the maximum allowed in the configuration.",
            }, status=HTTP_403_FORBIDDEN)
+


I had two possible approaches to implementing the proctoring specification.

write functions in api.py that a consuming application could use to create its own proctoring views

write views that implement the proctoring flow, which a consuming application would either install as is or that would be wrapped by views in the consuming application

I went with option #2 here. If I go with option #1, a lot is left up to the consuming application to ensure that the api functions are called correctly and in the right order. However, there does need to be a balance where a consuming application can customize the flow a little. In the case of edx-exams, the LtiConfiguration models are stored in that IDA; edx-exams should be able to specify what LtiConfiguration should be used based on its models. An even better example is attempt_number; xblock-lti-consumer will have no knowledge of exam attempts. This information must be passed to it, somehow.

For the sake of getting this working, I have decided that the consuming application will import and "wrap" these views, passing the appropriate context to these views. You can see an example in edx-exams here. That said, I'm not confident this approach solves all the needs of this library, so I'd like to discuss alternatives here.

the import renaming in exams is a bit of an indicator of weird stuff:
from lti_consumer.plugin.views import start_proctoring as lti_start_proctoring

would we expect any app to ever be able to use these views without wrapping? If not can we get any benefit by having them not be structured as views? They seem to all do request param copying and the exam wrapper also does that, so we make a copy of a copy to add things in layers. Maybe these functions could just take a dictionary of extra stuff to add. Maybe that could go all the way down into proctoring_preflight because all of its callers do the same setup

Do we absolutely need edx-exams to be providing the attempt_number and such? Could the UI just provide these values when starting the flow? In that way we can leave the views entirely within the lti-consumer library and avoid wrapping or using api functions right?

What would that look like? The launch_gate_endpoint_proctoring view should be invoked when the Tool makes a third party authentication request to the Platform (i.e. edx-exams). How would the frontend pass these data into the flow? I could see how the frontend could pass attempt_number when it kicks off the LTI flow (via the start_proctoring view) when, say, a learner presses a "start" button, but that data isn't needed the LTI flow is being kicked off. Once the flow is kicked off, will the frontend even be involved until it comes time to render content?

I'm going to call "LTI views" the endpoints from xblock-lti-consumer that are installed into edx-exams.

For the first request that is made, which is the third party login initiation (i.e. start_proctoring), the frontend could make a request to the LTI view that would include the LTI config_id. That makes sense to me. I assume the frontend would get it in some backend-for-frontend request before rendering the exam, right?

The Platform receives this request, grabs the LtiConfiguration using the config_id, crafts a preflight URL using the LtiProctoringConsumer, and returns a redirect to that preflight URL. The browser makes the request. Next, the Tool gets the request and makes a third party authentication request to the LTI view through the browser. edx-exams gets the request and has to issue an LTI launch. At this point, the LTI view needs to know attempt_number, resource_link, etc. in order to issue a launch. These data come from edx-exams, but the LTI view has no knowledge of them. I don't think the frontend can help here, unless I'm mistaken?

@ashultz0

would we expect any app to ever be able to use these views without wrapping? If not can we get any benefit by having them not be structured as views? They seem to all do request param copying and the exam wrapper also does that, so we make a copy of a copy to add things in layers. Maybe these functions could just take a dictionary of extra stuff to add. Maybe that could go all the way down into proctoring_preflight because all of its callers do the same setup

Unless we can find an alternative way to pass the necessary data around, I don't think so. I can see what that your suggestion would look like. You're envisioning that the input to any of these function would be a dictionary (not a request), and then the output would be a response object, right?

I think I meant instead of each layer passing in a request and a dict and combining the dict into the request params to make new request params we would just pass the request and ever increasing set of new params down to the lowest level before sticking them together and getting the response. The various layers all have the view interface but... is anyone ever really going to use the inner views as views? If not, they don't have the view argument set.

I'm going to call "LTI views" the endpoints from xblock-lti-consumer that are installed into edx-exams.

For the first request that is made, which is the third party login initiation (i.e. start_proctoring), the frontend could make a request to the LTI view that would include the LTI config_id. That makes sense to me. I assume the frontend would get it in some backend-for-frontend request before rendering the exam, right?

The Platform receives this request, grabs the LtiConfiguration using the config_id, crafts a preflight URL using the LtiProctoringConsumer, and returns a redirect to that preflight URL. The browser makes the request. Next, the Tool gets the request and makes a third party authentication request to the LTI view through the browser. edx-exams gets the request and has to issue an LTI launch. At this point, the LTI view needs to know attempt_number, resource_link, etc. in order to issue a launch. These data come from edx-exams, but the LTI view has no knowledge of them. I don't think the frontend can help here, unless I'm mistaken?

I guess I was under the impression that the contextual details such as attempt and resource_id wood be somehow be passed through the initial authentication steps. Otherwise once we do get to the step where the tool requests a launch from edx-exams how would edx exams even know what specific attempt this is for if it's only context at that point is a certain LtiConfiguration?

edit: can this be baked into the lti_message_hint? It looks to me like we could encode that as a JWT containing whatever state information we want.

They're not, unfortunately.

The specification doesn't detail how to accomplish this. It's a good question. I agree that it's not really clear how to get much more than the currently requesting user and the provider being used for the exam (from the client_id) when crafting the LTI launch message.

I believe that the way the existing LTI 1.3 launch gets around this is by being coupled to the XBlock location and by misusing the login_hint parameter. The login_hint parameter refers to a location on the LtiConfiguration, but it also lets the library pull data about the context of the LTI launch through the XBlock. I think that's why the full decoupling of the LTI views from LtiConfiguration.location has been challenging. It's a little bit like apples and oranges, but the context_claim is similar to attempt_number or resource_link. I considered whether we could abuse the login_hint to refer to an attempt to tie the two requests together, but it's really not the purpose of the parameter. Nevertheless, we could do that, but it doesn't address the other data that the library will need (e.g. resource_link, user role, etc.).

I'll think through some options tomorrow and report back.

MichaelRoytman · 2022-08-16T12:22:46Z

lti_consumer/api.py

@@ -142,25 +142,14 @@ def get_lti_1p3_launch_info(config_id=None, block=None):
    }


-def get_lti_1p3_launch_start_url(config_id=None, block=None, deep_link_launch=False, dl_content_id=None, hint=""):
+def get_lti_1p3_launch_start_url(config_id=None, block=None, lti_hint="", hint=""):


This makes it easy for the proctoring preflight view to generate the launch URL without adding another "proctoring" parameter to get_lti_1p3_launch_start_url. I thought it would be better for the caller to determine the appropriate lti_hint, but I can also see an argument that callers shouldn't concern themselves with the details of lti_hint (although hint is a parameter already). I didn't like the idea of needing to add additional variables for this function to determine the appropriate lti_hint.

would it be appropriate to change hint to login_hint? I think that makes the distinction between the parameters clearer.

edit: or possibly universally using message_hint instead of lti_hint if possible, the latter seems a bit vague to me.

I definitely support this. I found the distinction confusing myself and had to keep checking which was which. I'll just echo the names as they are in the specification: login_hint and lti_message_hint.

MichaelRoytman · 2022-08-16T12:56:42Z

lti_consumer/plugin/views.py

+            # before actually starting the assessment."
+            # See 3.3 Transferring the Candidate Back to the Assessment Platform.
+            # In the synchronizer token method of CSRF protection, the anti-CSRF token must be stored on the server.
+            session_data_key = get_cache_key(app="lti", key="session_data", user_id=request.user.id)


I have decided to use the TieredCache class to store the required anti-CSRF token session_data in memcached. Please see the comment above launch_gate_endpoint_proctoring for a detailed description of why I am not using the Django CSRF protection.

I have investigated alternatives to storing this value in memcached.

We could store session_data in request.session.

Because the requests from the Assessment Platform to the Proctoring Tool and vice versa are cross-origin, the browser will not include the session_id cookie that identifies the session unless I set the SESSION_COOKIE_SAMESITE Django setting to "None". This allows the browser to send session cookies as third-party cookies. This works, but it means that all session cookies in a consuming application or context will be sent this way, which is a big security risk. There isn't a way to specify that just one kind of session cookie behaves this way.

We could store session_data in a cookie.

The advantage of this is that we can specify that just this cookie can be sent cross-origin. However, this defeats the purpose of the synchronizer token pattern of CSRF protection. This would potentially leak the CSRF token. The session_data should be stored on the consuming context's side.

We could store session_data in the database.

request.session does this already, but the difficult part is getting access to the session object, since it's pulled out of the database by the session_id cookie. We could get around this by implementing our own model, but it seems like too heavy-handed when the memcached solution works just as well and handles cache invalidation.

MichaelRoytman · 2022-08-16T17:31:10Z

lti_consumer/plugin/views.py

@@ -93,7 +106,7 @@ def has_block_access(user, block, course_key):


 @require_http_methods(["GET"])
-def public_keyset_endpoint(request, usage_id=None):
+def public_keyset_endpoint(request, usage_id=None, lti_config_id=None):


This comes from #254.

MichaelRoytman · 2022-08-16T17:31:16Z

lti_consumer/plugin/views.py

@@ -149,22 +169,56 @@ def launch_gate_endpoint(request, suffix=None):

 @csrf_exempt
 @require_http_methods(["POST"])
-def access_token_endpoint(request, usage_id=None):
+def access_token_endpoint(request, lti_config_id=None, usage_id=None):


This comes from #254.

MichaelRoytman · 2022-08-16T17:32:41Z

lti_consumer/plugin/views.py

+    #           authenticity of the JWT and raise an exception if the signature is invalid later in this function.
+    token = request.POST.get('JWT')
+
+    # TODO: This needs better error handling.


I know the error handling in this file is inconsistent. I will plan on fixing it once we settle on an approach for implementing these views.

MichaelRoytman · 2022-08-16T18:49:31Z

lti_consumer/plugin/views.py

+    return JsonResponse(data={})
+
+
+# We do not want Django's CSRF protection enabled for POSTs made by external services to this endpoint. This is because


I was thinking this could warrant an ADR. Do you feel this comment is sufficient?

I don't think that django's CSRF not being able to handle this case is worth an ADR. It might be worth separating this note out into some readme if we have a suitable one (but I don't think we do)

I honestly would have just written "django's CSRF can't handle a flow where it isn't in control of the middle" so this is already more info than I demand :)

MichaelRoytman · 2022-08-16T19:54:04Z

lti_consumer/models.py

        if self.version == self.LTI_1P3 and self.config_store == self.CONFIG_ON_DB:
-            if not database_config_enabled(self.block.location.course_key):
+            if self.location and not database_config_enabled(self.block.location.course_key):


I'm going to pull this into a separate pull request, but I needed it here to get it working.

ashultz0 · 2022-08-17T20:09:20Z

lti_consumer/plugin/views.py

+    try:
+        lti_response = lti_consumer.check_and_decode_token(token)
+    # TODO: This needs better error handling.
+    except (BadJwtSignature, MalformedJwtToken, NoSuitableKeys):


a nonspecific 400 is an ok return for bad signature / malformed jwt both of which could be attacks

ashultz0 · 2022-08-17T20:51:45Z

lti_consumer/plugin/views.py

+    This uses the "client_id" query parameter or form data to identify the LtiConfiguration and its consumer to generate
+    the LTI 1.3 Launch Form.
+    """
+    preflight_response = request.GET.dict() if request.method == 'GET' else request.POST.dict()


this is super weird, there isn't a request method that just does this? request.THING_YOU_HAVE.dict()?

I couldn't find one, no ☹️ .

ashultz0 · 2022-08-17T20:58:21Z

lti_consumer/plugin/views.py

+            exc,
+            exc_info=True
+        )
+        return render(request, 'html/lti_1p3_launch_error.html', context, status=HTTP_400_BAD_REQUEST)


this function is pretty big and could do with some chunking, maybe pull the try: contents out into its own thing

…and dl_content parameters This commit modifies the get_lti_1p3_launch_start_url API method to remove the deep_link_launch and dl_content parameters. They are replaced with an lti_hint parameter. Callers of this function will need to supply the appropriate lti_hint. This commit also includes changes to callers of this method to include the appropriate lti_hint. This makes it easy for the proctoring preflight to generate the launch URL without adding another "proctoring" parameter to get_lti_1p3_launch_start_url.

…t proctoring

ashultz0 · 2022-08-18T14:19:05Z

lti_consumer/lti_1p3/consumer.py

+        """
+        Set the self.proctoring_data dictionary with the provided kwargs, so long as a given key is in
+        LTI_PROCTORING_DATA_KEYS.
+        """


If the kwargs are an incomplete set, what happens? Should this be more strongly typed with actual args?

I think I did it this way originally because I thought we'd need partial data in some places, but that's no longer the case. I'll change it back to something like set_proctoring_data(attempt_number, resource_link, ...).

ashultz0 · 2022-08-18T14:34:22Z

lti_consumer/lti_1p3/consumer.py

+                ' the picture claim should not be the same picture provided by the Assessment Platform to the Tool.'
+            )
+        # TODO: We can leverage these verified user claims. For example, we could use
+        #       these claims for Name Affirmation.


it might make sense to pull out the claim check in to a separate function which would be easier to test by itself

it could return the verified user we could just get that out again

Are you referring to lines 856 - 877?

MichaelRoytman · 2022-08-23T20:33:57Z

lti_consumer/lti_1p3/consumer.py

+        # Decode token and check expiration.
+        proctoring_response = self.tool_jwt.validate_and_decode(token)
+
+        # TODO: We MUST perform other forms of validation here.


I reread the specification, and now I cannot find where I read that we MUST do this validation. I swore I read it, but I cannot find it again. Based on that, I think the majority of this validation is not necessary. I may have misread a paragraph like the following as a requirement that we validate that the tool did its job. I don't think we need to do that. That would mean that the attempt_number and resource_link would not longer be needed by the start_assessment view.

4.3.1.6 Attempt number claim
The https://purl.imsglobal.org/spec/lti-ap/claim/attempt_number claim specifies the candidate's attempt number, as set by the Assessment Platform. The Proctoring Tool MUST copy this value to the Start Assessment message unchanged.

mphilbrick211 · 2022-10-26T02:57:04Z

@MichaelRoytman just checking in on this - are you planning to proceed with this PR? If so, you will need to re-run the tests. Thank you!

MichaelRoytman · 2022-10-26T16:14:02Z

@mphilbrick211 Thanks for following up. I do not plan to merge this pull request. I'll close it out. I have another pull request open that will take this pull request's place, and I have put it in review now. I'm hoping to merge it in soon.

MichaelRoytman force-pushed the mroytman/MST-1473-proctoring-service-launch branch 6 times, most recently from 97f50d5 to ccf639b Compare August 16, 2022 18:22

MichaelRoytman changed the title ~~-DRAFT: NOT READY FOR REVIEW- mroytman/MST-1473-proctoring-service-launch~~ [MST-1473] Proctoring Service Launch Implementation Aug 16, 2022

MichaelRoytman mentioned this pull request Aug 16, 2022

[MST-1473] Proctoring Service Launch edx/edx-exams#32

Merged

6 tasks

MichaelRoytman force-pushed the mroytman/MST-1473-proctoring-service-launch branch from ccf639b to edaaf11 Compare August 16, 2022 18:44

MichaelRoytman marked this pull request as ready for review August 16, 2022 18:47

MichaelRoytman commented Aug 16, 2022

View reviewed changes

ashultz0 reviewed Aug 17, 2022

View reviewed changes

MichaelRoytman added 12 commits August 17, 2022 17:17

feat: add proctoring enabled field to LTIConfiguration

ae93bf7

fix: grammar in exception message

0f9ccd9

fix: add additional error handling for invalid signature

20daae2

feat: add check_token_claim function for use by LtiProctoringConsumer

eeeb060

feat: add LtiProctoringConsumer

1d47431

feat: instantiate the LtiProctoringConsumer from the LtiConfiguration

452eb7e

feat: raise exception if LtiConfiguration config_store doesn't suppor…

5393bdd

…t proctoring

fix: bug prevent creation of LtiConfiguration if location is null

2e67152

feat: add views to plugin for Proctoring Services implementation

1174f91

feat: delete unused get_lti_proctoring_start_assessment_url

1bbe911

fix: remove unused imports from utils.py

979ca8f

MichaelRoytman force-pushed the mroytman/MST-1473-proctoring-service-launch branch from edaaf11 to 979ca8f Compare August 17, 2022 21:17

ashultz0 reviewed Aug 18, 2022

View reviewed changes

MichaelRoytman commented Aug 23, 2022

View reviewed changes

MichaelRoytman closed this Oct 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MST-1473] Proctoring Service Launch Implementation #275

[MST-1473] Proctoring Service Launch Implementation #275

MichaelRoytman commented Aug 15, 2022 •

edited

Loading

MichaelRoytman Aug 15, 2022

zacharis278 Aug 22, 2022

MichaelRoytman Aug 15, 2022

MichaelRoytman Aug 15, 2022

ashultz0 Aug 17, 2022

zacharis278 Aug 23, 2022

MichaelRoytman Aug 23, 2022

MichaelRoytman Aug 23, 2022

MichaelRoytman Aug 23, 2022

ashultz0 Aug 23, 2022

zacharis278 Aug 23, 2022 •

edited

Loading

MichaelRoytman Aug 23, 2022

MichaelRoytman Aug 16, 2022

zacharis278 Aug 22, 2022 •

edited

Loading

MichaelRoytman Aug 23, 2022

MichaelRoytman Aug 16, 2022

MichaelRoytman Aug 16, 2022

MichaelRoytman Aug 16, 2022

MichaelRoytman Aug 16, 2022

MichaelRoytman Aug 16, 2022

ashultz0 Aug 17, 2022

MichaelRoytman Aug 16, 2022

ashultz0 Aug 17, 2022

ashultz0 Aug 17, 2022

MichaelRoytman Aug 23, 2022

ashultz0 Aug 17, 2022

ashultz0 Aug 18, 2022

MichaelRoytman Aug 23, 2022

ashultz0 Aug 18, 2022

MichaelRoytman Aug 23, 2022

ashultz0 Aug 23, 2022

MichaelRoytman Aug 23, 2022

mphilbrick211 commented Oct 26, 2022

MichaelRoytman commented Oct 26, 2022

		return JsonResponse(data={})


		# We do not want Django's CSRF protection enabled for POSTs made by external services to this endpoint. This is because

[MST-1473] Proctoring Service Launch Implementation #275

[MST-1473] Proctoring Service Launch Implementation #275

Conversation

MichaelRoytman commented Aug 15, 2022 • edited Loading

Description

Key Details

Dependencies

Testing Instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zacharis278 Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zacharis278 Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mphilbrick211 commented Oct 26, 2022

MichaelRoytman commented Oct 26, 2022

MichaelRoytman commented Aug 15, 2022 •

edited

Loading

zacharis278 Aug 23, 2022 •

edited

Loading

zacharis278 Aug 22, 2022 •

edited

Loading