Add support for OpenId Connect userinfo_endpoint #4649

stephen-crawford · 2024-08-15T20:49:43Z

Description

[Describe what this change achieves]
This change reworks the OpenID Connect authentication process to allow for the configuration of the new setting userinfo_endpoint. It also allows the configuration of the two associated settings client_id and issuer_id_url, which are used when validating the response provided by the OIDC provider back to the client.

The userinfo_endpoint is a configuration of OIDC originally detailed in the RFC: https://openid.net/specs/openid-connect-core-1_0.html#UserInfo

It functions by creating an automated callback between the Client and the OIDC provider. In the RFC, these are referred to as the Relying Party (RP) and OpenID Provider (OP). In short while the existing OIDC support works by causing the OP to simply send a complete JWT to be parsed by OpenSearch, the new support works by returning that JWT back to the OP while querying the userinfo_endpoint configured during registration.

In turn, the OP will return either a serialized JSON response or a new JWT. This response is then validated based on the details outlined in the RFC, before the sub and roles fields of either the JSON or JWT are formed into a new set of AuthCredentials.

Here is an example of how this works

OP --/app/home# Bearer <sub: stephen>--> RP
RP --GET /<userinfo_endpoint> Bearer <sub: stephen--> OP
OP --OK <sub: stephen, roles: admin, iss: ...> --> RP

Issues Resolved

This change should resolve: #2040

Is this a backport? If so, please add backport PR # and/or commits #, and remove backport-failed label from the original PR.

Do these changes introduce new permission(s) to be displayed in the static dropdown on the front-end? If so, please open a draft PR in the security dashboards plugin and link the draft PR here

Testing

I have updated the existing OIDC tests and added new tests which cover the JWT and JSON flows for the userinfo behavior.

Check List

New functionality includes testing
New functionality has been documented
New Roles/Permissions have a corresponding security dashboards plugin PR
API changes companion pull request created
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2024-08-15T21:14:56Z

Codecov Report

Attention: Patch coverage is 74.13793% with 30 lines in your changes missing coverage. Please review.

Project coverage is 65.24%. Comparing base (0b66f48) to head (ca57141).
Report is 75 commits behind head on main.

Files with missing lines	Patch %	Lines
...th/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java	78.78%	12 Missing and 9 partials ⚠️
...ic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java	53.84%	3 Missing and 3 partials ⚠️
.../dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java	0.00%	2 Missing ⚠️
.../dlic/auth/http/jwt/keybyoidc/OpenIdConstants.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4649      +/-   ##
==========================================
- Coverage   65.26%   65.24%   -0.03%     
==========================================
  Files         317      318       +1     
  Lines       22320    22398      +78     
  Branches     3591     3603      +12     
==========================================
+ Hits        14568    14614      +46     
- Misses       5953     5978      +25     
- Partials     1799     1806       +7

Files with missing lines	Coverage Δ
...azon/dlic/auth/http/jwt/keybyoidc/JwtVerifier.java	`80.00% <ø> (ø)`
...arch/security/securityconf/DynamicConfigModel.java	`100.00% <100.00%> (ø)`
...va/org/opensearch/security/support/ModuleType.java	`87.50% <ø> (ø)`
.../dlic/auth/http/jwt/keybyoidc/OpenIdConstants.java	`0.00% <0.00%> (ø)`
.../dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java	`77.57% <0.00%> (-1.48%)`	⬇️
...ic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java	`60.43% <53.84%> (ø)`
...th/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java	`78.78% <78.78%> (ø)`

... and 4 files with indirect coverage changes

stephen-crawford · 2024-08-21T15:20:55Z

Opening this up for review while I go ahead and try to test the use case. Going to use OpenSearch with the custom installation and Keycloak as the OIDC provider.

cwperks · 2024-08-21T15:33:54Z

...ain/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPJwtKeyByOpenIdConnectAuthenticator.java

@@ -1,73 +0,0 @@
-/*


@stephen-crawford Is the user-info endpoint only used to extract the backend roles from the external IdP or does it have another use as well? (i.e. custom attributes)

Would it make sense to have a protected method in the AbstractHttpJwtAuthenticator to extractRoles and have this class override the implementation?

The class renaming is making it harder to determine what the diffs are in this PR.

It can contain anything so long as it minimally contains the sub field. If it is signed, it must also contain a iss and aud field.

The renaming was done because the old naming convention was inconsistent with the rest of the Http authenticators.

The existing code was the nested class now inside the HTTPOpenIdAuthenticator file and the associated tests. The rest is new.

Got it, I see an example positive response here: https://docs.aws.amazon.com/cognito/latest/developerguide/userinfo-endpoint.html#get-userinfo-response-sample

So userinfo endpoint needs to be called before extractRoles and extractSubject.

Yeah, this code is pretty complicated which is why I tried to add a lot of tests. The issue is that the existing OIDC logic was not implemented with the userinfo endpoint in mind. It is tightly coupled to the JWTAuthenticator (and the abstract version), but the userinfo can be a jwt or json which complicates things.

@cwperks I added a comment on the test file and the renamed file to help make clear what is new.

cwperks · 2024-08-21T15:37:14Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+    }
+
+    public HTTPJwtKeyByOpenIdConnectAuthenticator getOpenIdJwtAuthenticator() {
+        if (openIdJwtAuthenticator == null) {


What's the purpose of synchronized here and multiple null checks?

I made this for the case where we want to access the wrapped jwtAuthenticator. If the overall Authenticator is not initialized, we would want to be able to access the internal authenticator for testing I believe. The synchronized is meant to prevent the creation of multiple authenticators when getOpenIdJwtAuthenticator is called multiple times. We don't want to create several instances of the jwtOpenIdAuthenticator so this makes sure we only ever make one.

If you think there is a better way of doing this let me know.

Singleton Pattern should help here correct?

I am going to remove this since apparently this pattern is not recommended in modern coding practices ...

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/OpenIdConstants.java

stephen-crawford · 2024-08-21T15:49:53Z

src/test/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticatorTests.java

+    }
+
+    @Test
+    public void userinfoEndpointReturnsJwtWithAllRequirementsTest() throws Exception {


This point starts the new tests.

a lot of code is repetitive in the test below. Do you think we can extract it to a single method, say assertValidCredentials and assertInvalidCredentials?

cwperks · 2024-08-21T15:50:56Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+
+                String userinfoContent;
+
+                try (


Would it be possible to use nimbus' UserInfoResponse.parse(response)?

The KeySetRetriever use's nimbuses similarly like this: https://github.com/opensearch-project/security/blob/main/src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/KeySetRetriever.java#L92-L97

Hmm let me look. I just googled how to read the entity content properly but if we already have something that does that, then that would be great.

So, I took a look and I think trying to use the package is more hassle than its worth. For starters, we need to add a dependency on https://mvnrepository.com/artifact/com.nimbusds/oauth2-oidc-sdk. But more notably, trying to use the Parse logic will require resolving a translation between the CloseableHttpResponse we current use and the generic HttpResponse that the built in method expects.

It seems like we are probably better off leaving things as they are for now.

stephen-crawford · 2024-08-21T15:51:03Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+            return getJwtClaimsSetFromInfoContent(parsedToken);
+        }
+
+        private JWTClaimsSet getJwtClaimsSetFromInfoContent(String userInfoContent) throws OpenSearchSecurityException {


This is new--how I split it out from the extract method--but the rest of the nested class was previously implemented.

stephen-crawford · 2024-08-21T15:56:19Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+        return missing;
+    }
+
+    private final class HTTPJwtKeyByOpenIdConnectAuthenticator extends AbstractHTTPJwtAuthenticator {


All above is new. This internal class previously existed as the separate HttpJwtKeyByOpenIdConnectAuthenticator. I nested the class since the userinfo logic directly relies on it and the previous naming was out of convention.

DarshitChanpura

Thank you @stephen-crawford for adding this support. Changes look close. Left a few comments. Could you look into codecov annotations about missed lines in test coverage?

DarshitChanpura · 2024-08-22T17:52:23Z

build.gradle

@@ -608,7 +608,8 @@ dependencies {
    runtimeOnly 'org.ow2.asm:asm:9.7'

    testImplementation 'org.apache.camel:camel-xmlsecurity:3.22.2'
-
+    testImplementation 'org.mockito:mockito-inline:5.2.0'
+    //testImplementation 'org.mockito:mockito-inline:2.13.0'


nit: remove this commented line.

DarshitChanpura · 2024-08-22T17:54:52Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+    }
+
+    public HTTPJwtKeyByOpenIdConnectAuthenticator getOpenIdJwtAuthenticator() {
+        if (openIdJwtAuthenticator == null) {


Singleton Pattern should help here correct?

DarshitChanpura · 2024-08-22T18:36:13Z

src/test/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticatorTests.java

+    }
+
+    @Test
+    public void userinfoEndpointReturnsJwtWithAllRequirementsTest() throws Exception {


a lot of code is repetitive in the test below. Do you think we can extract it to a single method, say assertValidCredentials and assertInvalidCredentials?

stephen-crawford · 2024-08-22T20:41:53Z

Hi @DarshitChanpura, thanks for taking a look. I will definitely address your comments before merging. Right now, I am having to backport these changes to the 2.x line to check the stability and functionality of the feature so it will be tomorrow or next week before I update--but I am going to.

Signed-off-by: Stephen Crawford <[email protected]>

stephen-crawford · 2024-08-26T20:33:41Z

@cwperks I swapped over to using Nimbus for sending the requests like you suggested. I ran into some problems with the verification of the fields since it seems like the Nimbus validation is based off of the server response not the actual fields equaling the settings. I also addressed @DarshitChanpura's comments while takng care of this swap and revising the tests.

Going to go back to manually testing things now.

nibix · 2024-08-27T12:23:46Z

@stephen-crawford Cool feature! I am wondering about one thing: Is the userinfo endpoint then queried for each incoming REST request? If so, this might add quite a bit of latency on these requests.

stephen-crawford · 2024-08-27T13:51:05Z

Hi @nibix, to answer your question, yes. The server will need to be queried for each request. There will be some additional latency when using this feature but presumably anyone who wants to use it would be aware of this trade off. I don't think there is an alternative to this approach since I don't believe our OIDC support caches the credentials for a session. This is a possible improvement down the line but from what I can tell, we expect each request to include valid authorization info. If we were to add caching of some kind, I would expect a new setting i.e. userinfo_ttl which would let you configure the time length that the userinfo should remain valid for. Then a temporary credential could be provided upon the first credentials fetch and returned back to OpenSearch upon all subsequent requests inside that time period.

This would require a fair amount of additional work I suspect.

nibix · 2024-08-27T14:33:17Z

@stephen-crawford There's already a user cache in BackendRegistry:

security/src/main/java/org/opensearch/security/auth/BackendRegistry.java

Lines 580 to 590 in 359272e

    
           // noop backend configured and no authorizers 
        
           // that mean authc and authz was completely done via HTTP (like JWT or PKI) 
        
           if (authBackend.getClass() == NoOpAuthenticationBackend.class && authorizers.isEmpty()) { 
        
               // no cache 
        
               return authBackend.authenticate(ac); 
        
           } 
        
           return cache.get(ac, new Callable<User>() { 
        
               @Override 
        
               public User call() throws Exception { 
        
                   if (log.isTraceEnabled()) {

Currently, the user cache is only used when there is a AuthenticationBackend which is non no-op or if there are AuthorizationBackend instances. Following the comment in the code, it looks like that the original assumption was that classes implementing HTTPAuthenticator work only on the request object itself, i.e., not communicating with further backends. That would be the motivation to disable the cache in these cases - as it happens in the code.

To a certain extend, one could argue that the user_info endpoint can be also considered as an authentication backend. If it would be used as such, the user cache would automatically get active - thus reducing the number of necessary requests to the user_info endpoint.

stephen-crawford · 2024-08-27T14:41:30Z

Hi @nibix, my understanding from the comment in the linked code is that the cache is not active for OIDC. It reads

 // noop backend configured and no authorizers 
 // that mean authc and authz was completely done via HTTP (like JWT or PKI) 
 if (authBackend.getClass() == NoOpAuthenticationBackend.class && authorizers.isEmpty()) { 
     // no cache 
     return authBackend.authenticate(ac); 
 }

So since OIDC is based on the JWT auth I interpreted this as meaning there would not be any caching here. Perhaps, I am mistaken.

Either way, it sounds like you have reservations about making this change? Is there something you would like to see done to this PR to move forward? I still need to setup the test systems to actually confirm it works as expected but the code should mostly be complete.

nibix · 2024-08-27T14:50:49Z

Exactly, my point was that there is no caching done for OIDC because it is assumed to be fast, i.e., just operating on locally available information. The original assumption is IMHO that "slow" operations are done in the AuthenticationBackend instaces - thus the cache is enabled there.

But, I don't want to interfere here, especially, as the PR is already in an advanced state, just wanted to add some perspective.

Signed-off-by: Stephen Crawford <[email protected]>

cwperks · 2024-08-27T15:16:29Z

A round trip per request is a lot of overhead and keeping the user cache in sync across nodes would also be a challenging problem.

Ideally, there is a token that has claims about the subject and roles that can be extracted after initial authentication so that only a round trip on creation of a session is necessary.

Edit: Since OIDC is used for dashboards auth, the increase in latency may be tolerable for users that opt into this feature

Signed-off-by: Stephen Crawford <[email protected]>

cwperks · 2024-08-27T15:30:02Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+
+        String missing = "";
+
+        if (claims.getClaim(SUB_CLAIM) == null || claims.getClaim(SUB_CLAIM).toString().isBlank() || !claims.getClaim("sub").equals(id)) {


This should also support the subject_key and roles_key settings: https://opensearch.org/docs/latest/security/authentication-backends/openid-connect/#configure-openid-connect-integration

It is not the same. The userinfo endpoint has to have the 'sub' field specifically.

"The sub (subject) Claim MUST always be returned in the UserInfo Response.

NOTE: Due to the possibility of token substitution attacks (see Section 16.11), the UserInfo Response is not guaranteed to be about the End-User identified by the sub (subject) element of the ID Token. The sub Claim in the UserInfo Response MUST be verified to exactly match the sub Claim in the ID Token; if they do not match, the UserInfo Response values MUST NOT be used."

https://openid.net/specs/openid-connect-core-1_0.html#UserInfo

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

cwperks · 2024-08-27T15:36:12Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+
+            String finalBearerHeader = bearerHeader;
+
+            AccessToken accessToken = new AccessToken(AccessTokenType.BEARER, finalBearerHeader) {


FYI Nimbus has a class called BearerAccessToken, would that be usable here?

Looks like this is the source code: https://bitbucket.org/connect2id/oauth-2.0-sdk-with-openid-connect-extensions/src/c7736eb688bf0a81714374822d4e7b5232222eea/src/main/java/com/nimbusds/oauth2/sdk/token/BearerAccessToken.java#lines-208:211

Oh I didn't realize that.

cwperks · 2024-08-27T15:42:08Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+                    );
+                }
+
+                String contentType = String.valueOf(httpResponse.getHeaderValues("content-type"));


Extract out the UserInfoSuccessResponse here and then call on UserInfoSuccessResponse.getEntityContentType to get the content type from the request.

cwperks · 2024-08-27T16:03:46Z

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java

+
+    @Override
+    public AuthCredentials extractCredentials(SecurityRequest request, ThreadContext context) throws OpenSearchSecurityException {
+        if (this.userInfoEndpoint != null && !this.userInfoEndpoint.isBlank()) {


wdyt about moving the logic that calls the userinfo endpoint into a new method?

In AbstractHttpJwtAuthenticator would it make sense to add a new method called:

public JwtClaimsSet getJWTClaimsSet(String jwt) { return jwt.getJWTClaimsSet(); // default implementation }

That way this line can be replaced: https://github.com/opensearch-project/security/blob/main/src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java#L131

In the OpenID authenticator can we then subclass that and either return jwt.getJWTClaimsSet() (if userinfo endpoint is not configured) or if userinfo is configured then follow the logic that you have below.

I am not sure what the benefit of further moving things would be. We already have separation of the userinfo vs. non-userinfo logic inside this handler. If you really prefer it that way, I can move things around, but don't see how

@Override public AuthCredentials extractCredentials(SecurityRequest request, ThreadContext context) throws OpenSearchSecurityException { return new AuthCredentials(this.getJwtClaims()) } public JwtClaimsSet getJwtClaims() { if (this.userInfoEndpoint != null && !this.userInfoEndpoint.isBlank()) { return extractCredentials0(request, context); } return (this.openIdJwtAuthenticator.extractCredentials(request, context)); }

is better than

@Override public AuthCredentials extractCredentials(SecurityRequest request, ThreadContext context) throws OpenSearchSecurityException { if (this.userInfoEndpoint != null && !this.userInfoEndpoint.isBlank()) { return extractCredentials0(request, context); } return (this.openIdJwtAuthenticator.extractCredentials(request, context)); }

These are functionally identical in my opinion and make the code harder to follow I feel. You can decide what you prefer though.

This class would not need to override extractCredentials, it would completely re-use extractCredentials from AbstractHTTPJwtAuthenticator, but override the implementation of getJWTClaimsSet.

Signed-off-by: Stephen Crawford <[email protected]>

DarshitChanpura · 2024-11-12T04:09:40Z

@cwperks @derek-ho How should we proceed with this one?

stephen-crawford marked this pull request as ready for review August 21, 2024 15:20

stephen-crawford requested review from cliu123, cwperks, DarshitChanpura, peternied, RyanL1997, reta and willyborankin as code owners August 21, 2024 15:20

cwperks reviewed Aug 21, 2024

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java Outdated Show resolved Hide resolved

cwperks reviewed Aug 21, 2024

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java Outdated Show resolved Hide resolved

cwperks reviewed Aug 21, 2024

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/OpenIdConstants.java Outdated Show resolved Hide resolved

cwperks reviewed Aug 21, 2024

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/OpenIdConstants.java Outdated Show resolved Hide resolved

stephen-crawford commented Aug 21, 2024

View reviewed changes

cwperks reviewed Aug 21, 2024

View reviewed changes

stephen-crawford commented Aug 21, 2024

View reviewed changes

DarshitChanpura reviewed Aug 22, 2024

View reviewed changes

stephen-crawford added 8 commits August 26, 2024 10:45

OIDC UserInfo Endpoint

1a3e5d9

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

1c0344a

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

e300d7d

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

7593421

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

3b9ef8c

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

6df0d02

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

9115585

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

6497003

Signed-off-by: Stephen Crawford <[email protected]>

stephen-crawford added 7 commits August 26, 2024 10:45

OIDC Userinfo Endpoint

2f4f065

Signed-off-by: Stephen Crawford <[email protected]>

OIDC Userinfo Endpoint Support

e0c5c17

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

8450932

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

435c099

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

1684db1

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

b601a92

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

c837dcf

Signed-off-by: Stephen Crawford <[email protected]>

stephen-crawford force-pushed the oidcUserInfo branch from 130cd6c to c837dcf Compare August 26, 2024 19:41

stephen-crawford requested a review from nibix as a code owner August 26, 2024 19:41

OIDC UserInfo Endpoint

3fbf595

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

fd8dd4c

Signed-off-by: Stephen Crawford <[email protected]>

stephen-crawford added 2 commits August 27, 2024 11:22

OIDC UserInfo Endpoint

8e25e32

Signed-off-by: Stephen Crawford <[email protected]>

OIDC UserInfo Endpoint

ec73fd2

Signed-off-by: Stephen Crawford <[email protected]>

cwperks reviewed Aug 27, 2024

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java Outdated Show resolved Hide resolved

cwperks reviewed Aug 27, 2024

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/keybyoidc/HTTPOpenIdAuthenticator.java Outdated Show resolved Hide resolved

cwperks reviewed Aug 27, 2024

View reviewed changes

OIDC UserInfo Endpoint

ca57141

Signed-off-by: Stephen Crawford <[email protected]>


		String missing = "";

		if (claims.getClaim(SUB_CLAIM) == null \|\| claims.getClaim(SUB_CLAIM).toString().isBlank() \|\| !claims.getClaim("sub").equals(id)) {


		String finalBearerHeader = bearerHeader;

		AccessToken accessToken = new AccessToken(AccessTokenType.BEARER, finalBearerHeader) {

Add support for OpenId Connect userinfo_endpoint #4649

Are you sure you want to change the base?

Add support for OpenId Connect userinfo_endpoint #4649

Conversation

stephen-crawford commented Aug 15, 2024 • edited Loading

Description

Issues Resolved

Testing

Check List

codecov bot commented Aug 15, 2024 • edited Loading

Codecov Report

stephen-crawford commented Aug 21, 2024 • 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

DarshitChanpura left a 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

stephen-crawford commented Aug 22, 2024

stephen-crawford commented Aug 26, 2024

nibix commented Aug 27, 2024

stephen-crawford commented Aug 27, 2024

nibix commented Aug 27, 2024 • edited Loading

stephen-crawford commented Aug 27, 2024

nibix commented Aug 27, 2024

cwperks commented Aug 27, 2024 • 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

cwperks Aug 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarshitChanpura commented Nov 12, 2024

stephen-crawford commented Aug 15, 2024 •

edited

Loading

codecov bot commented Aug 15, 2024 •

edited

Loading

stephen-crawford commented Aug 21, 2024 •

edited

Loading

nibix commented Aug 27, 2024 •

edited

Loading

cwperks commented Aug 27, 2024 •

edited

Loading

cwperks Aug 27, 2024 •

edited

Loading