Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Adds support for carrying Windows end-user authentication (EUA) context from the MDM-triggered Fleet MSI install into Orbit enrollment, aiming to prevent users from being prompted to authenticate twice.
Changes:
- Generate and embed a Fleet-signed EUA JWT (UPN + Windows MDM device ID) into the Windows MDM MSI install command line.
- Extend Orbit enrollment API/service plumbing to accept
eua_tokenand use it to link IdP/SCIM context during Windows enrollment when EUA is enabled. - Extend WSTEP STS token implementation to include a
device_idclaim and add unit tests for the new claim handling.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/service/orbit.go | Accepts EUA token during Orbit enrollment and performs Windows-specific EUA token processing + post-enrollment linking/mapping. |
| server/service/microsoft_mdm.go | Generates EUA token and appends it as EUA_TOKEN to the Windows MDM MSIInstallJob command line. |
| server/mock/service/service_mock.go | Updates mock service signature for EnrollOrbit to include euaToken. |
| server/mdm/microsoft/wstep.go | Adds device_id to STS claims and introduces helpers to mint/parse tokens including device ID. |
| server/mdm/microsoft/wstep_test.go | Adds tests covering token generation/validation with and without device_id. |
| server/fleet/service.go | Updates Fleet service interface EnrollOrbit signature to include euaToken. |
| server/fleet/api_orbit.go | Extends Orbit enroll request payload with optional eua_token. |
| changes/41381-eua-ms-installer | User-visible changelog entry for EUA context support in Fleet MSI installer flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #43295 +/- ##
==========================================
- Coverage 66.91% 66.90% -0.01%
==========================================
Files 2596 2596
Lines 208103 208220 +117
Branches 9321 9321
==========================================
+ Hits 139248 139314 +66
- Misses 56199 56235 +36
- Partials 12656 12671 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-user authentication (EUA) token support across Windows MDM and Orbit enrollment. Introduces 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/fleet/service.go (1)
120-125:⚠️ Potential issue | 🟡 MinorUpdate
EnrollOrbitcomment to includeeuaTokensemantics.The interface signature now takes
euaToken, but the method docs still describe only host info + enroll secret.✏️ Suggested doc fix
- // EnrollOrbit enrolls an orbit instance to Fleet by using the host information + enroll secret + // EnrollOrbit enrolls an orbit instance to Fleet by using the host information + enroll secret. + // euaToken is an optional Fleet-signed end-user auth context token used in Windows MDM EUA flows. // and returns the orbit node key if successful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/fleet/service.go` around lines 120 - 125, The doc comment for EnrollOrbit is outdated: it mentions only host information + enroll secret but the signature includes the euaToken parameter; update the comment for EnrollOrbit to describe the semantics of euaToken (what it represents, when it is used, and how it affects enrollment), and mention that the method takes hostInfo, enrollSecret, and euaToken and returns the orbit node key or an error; locate the comment above the EnrollOrbit interface declaration and add a concise sentence explaining euaToken's purpose and behavior alongside the existing bullets about host creation/update.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/mdm/microsoft/wstep.go`:
- Around line 218-240: The NewSTSAuthTokenWithDeviceID helper currently allows
an empty deviceID (STSClaims.DeviceID is omitempty) which results in signing
tokens without a device_id; add a validation at the start of
NewSTSAuthTokenWithDeviceID to return an error when deviceID == "" (similar to
the existing upn check) so token generation fails fast; update any unit tests
that call NewSTSAuthTokenWithDeviceID to expect an error for empty deviceID.
In `@server/service/orbit.go`:
- Around line 302-319: The SCIM mapping logic is currently only executed when
the boolean updated is true; instead, run the ScimUserByUserNameOrEmail ->
SetOrUpdateHostSCIMUserMapping/DeleteHostSCIMUserMapping flow whenever EUA
validation succeeds regardless of updated. Remove or relocate the updated gate
so that after EUA token validation (the point where euaUPN is available and
validation is confirmed) you always call svc.ds.ScimUserByUserNameOrEmail(ctx,
euaUPN, euaUPN) and then either svc.ds.SetOrUpdateHostSCIMUserMapping(ctx,
host.ID, scimUser.ID) or svc.ds.DeleteHostSCIMUserMapping(ctx, host.ID) with the
same error handling as today (keep checks for fleet.IsNotFound and sql.ErrNoRows
and the logger messages).
- Around line 119-146: The current code only calls
svc.ds.AssociateHostMDMIdPAccountDB when device.HostUUID == "" which skips
creating the host↔IdP mapping for reenroll/reinstall cases; move or refactor the
mdm idp account lookup/creation (calls to svc.ds.GetMDMIdPAccountByEmail and
svc.ds.InsertMDMIdPAccount and the re-fetch into acct) so that acct is ensured
regardless of device.HostUUID, then always call
svc.ds.AssociateHostMDMIdPAccountDB(ctx, hostUUID, acct.UUID) (using the
existing hostUUID/upn variables) to create the host_mdm_idp_accounts link even
when device.HostUUID is populated; keep the same error wrapping
(ctxerr.Wrap/New) around each DB call.
---
Outside diff comments:
In `@server/fleet/service.go`:
- Around line 120-125: The doc comment for EnrollOrbit is outdated: it mentions
only host information + enroll secret but the signature includes the euaToken
parameter; update the comment for EnrollOrbit to describe the semantics of
euaToken (what it represents, when it is used, and how it affects enrollment),
and mention that the method takes hostInfo, enrollSecret, and euaToken and
returns the orbit node key or an error; locate the comment above the EnrollOrbit
interface declaration and add a concise sentence explaining euaToken's purpose
and behavior alongside the existing bullets about host creation/update.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a13d5875-4fd9-4d12-8769-4e5f40a9ccdd
📒 Files selected for processing (8)
changes/41381-eua-ms-installerserver/fleet/api_orbit.goserver/fleet/service.goserver/mdm/microsoft/wstep.goserver/mdm/microsoft/wstep_test.goserver/mock/service/service_mock.goserver/service/microsoft_mdm.goserver/service/orbit.go
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
server/service/orbit.go:265
- The Windows EUA token path introduces several new side effects (token validation, IdP account creation/linking, mdm_windows_enrollments host_uuid update, SCIM mapping updates) but there are no tests in this PR that exercise the end-to-end orbit enrollment behavior when
eua_tokenis present/invalid. Given there are existing integration tests around/api/fleet/orbit/enroll, consider adding coverage that asserts: (1) when end-user auth is required and a valid EUA token is provided, enrollment does not return END_USER_AUTH_REQUIRED; and (2) invalid/expired tokens fall back to END_USER_AUTH_REQUIRED without linking/mapping changes.
} else if platform == "windows" && euaToken != "" {
// A Windows host already authenticated during MDM enrollment and the
// EUA token was passed by the MSI installer.
upn, deviceID, err := svc.processWindowsEUAToken(ctx, hostInfo.HardwareUUID, euaToken)
if err != nil {
return "", err
}
euaUPN = upn
euaDeviceID = deviceID
// Continue enrollment — do not return END_USER_AUTH_REQUIRED.
} else {
// Otherwise report the unauthenticated host and let Orbit handle it (e.g. by prompting the user to authenticate).
return "", fleet.NewOrbitIDPAuthRequiredError()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
server/service/orbit_eua_test.go (3)
149-151: Tighten mock expectations by assertingmdmDeviceIDin all datastore callbacks.Several callbacks ignore the
mdmDeviceIDargument. That can hide regressions where the wrong device ID is passed through.♻️ Suggested test hardening
ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { + require.Equal(t, testDeviceID, mdmDeviceID) return &fleet.MDMWindowsEnrolledDevice{MDMDeviceID: testDeviceID, HostUUID: ""}, nil }Apply the same assertion pattern to the other
MDMWindowsGetEnrolledDeviceWithDeviceIDFuncstubs in this file.Also applies to: 171-174, 223-225, 245-247, 263-265, 274-276, 285-287, 296-298
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/orbit_eua_test.go` around lines 149 - 151, Several datastore mock callbacks for MDMWindowsGetEnrolledDeviceWithDeviceIDFunc aren't asserting the mdmDeviceID argument; tighten these by verifying the incoming mdmDeviceID equals the expected testDeviceID and fail the test if not. Update every stub of MDMWindowsGetEnrolledDeviceWithDeviceIDFunc in this file (including the ones around the indicated ranges) to check mdmDeviceID == testDeviceID before returning the fleet.MDMWindowsEnrolledDevice, so the mock will catch wrong IDs being passed through.
202-203: Use a local constant forEND_USER_AUTH_REQUIREDin assertions.Repeated string literals make tests more fragile to message renames and create duplication.
🧹 Small cleanup
func TestProcessWindowsEUAToken(t *testing.T) { + const endUserAuthRequired = "END_USER_AUTH_REQUIRED" const ( testUPN = "user@example.com" testDeviceID = "device-abc-123" ... ) ... - require.Equal(t, "END_USER_AUTH_REQUIRED", orbitErr.Message) + require.Equal(t, endUserAuthRequired, orbitErr.Message)Also applies to: 214-215, 231-232
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/orbit_eua_test.go` around lines 202 - 203, Define a local constant (e.g., expectedErrMsg := "END_USER_AUTH_REQUIRED") at the start of the test and replace the repeated string literals in the require.Equal assertions with this constant (locations: the assert comparing orbitErr.Message and the two other asserts around lines testing the same message), keeping the rest of the assertions (like require.False(t, ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFuncInvoked)) unchanged so the test uses a single source of truth for the expected error message.
194-204: Add a dedicated expired-token test.This case currently validates a malformed token; an explicit expired-but-well-formed token case would better protect expiry handling from regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/orbit_eua_test.go` around lines 194 - 204, Add a new test case that mirrors the malformed-token case but uses a well-formed JWT with an expiration (exp) claim set in the past to assert expiry handling; in the test (e.g., t.Run("expired token falls back to END_USER_AUTH_REQUIRED", ...)) create a valid-looking JWT string with a past exp, initialize ds := new(mock.Store) and svc := newTestServiceWithWSTEP(t, ds), call svc.processWindowsEUAToken(context.Background(), testHostUUID, "<expired-jwt>"), require an error and that it is an *fleet.OrbitError with Message "END_USER_AUTH_REQUIRED", and assert ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFuncInvoked is false so expiry behavior is covered separately from malformed-token handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@server/service/orbit_eua_test.go`:
- Around line 149-151: Several datastore mock callbacks for
MDMWindowsGetEnrolledDeviceWithDeviceIDFunc aren't asserting the mdmDeviceID
argument; tighten these by verifying the incoming mdmDeviceID equals the
expected testDeviceID and fail the test if not. Update every stub of
MDMWindowsGetEnrolledDeviceWithDeviceIDFunc in this file (including the ones
around the indicated ranges) to check mdmDeviceID == testDeviceID before
returning the fleet.MDMWindowsEnrolledDevice, so the mock will catch wrong IDs
being passed through.
- Around line 202-203: Define a local constant (e.g., expectedErrMsg :=
"END_USER_AUTH_REQUIRED") at the start of the test and replace the repeated
string literals in the require.Equal assertions with this constant (locations:
the assert comparing orbitErr.Message and the two other asserts around lines
testing the same message), keeping the rest of the assertions (like
require.False(t, ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFuncInvoked))
unchanged so the test uses a single source of truth for the expected error
message.
- Around line 194-204: Add a new test case that mirrors the malformed-token case
but uses a well-formed JWT with an expiration (exp) claim set in the past to
assert expiry handling; in the test (e.g., t.Run("expired token falls back to
END_USER_AUTH_REQUIRED", ...)) create a valid-looking JWT string with a past
exp, initialize ds := new(mock.Store) and svc := newTestServiceWithWSTEP(t, ds),
call svc.processWindowsEUAToken(context.Background(), testHostUUID,
"<expired-jwt>"), require an error and that it is an *fleet.OrbitError with
Message "END_USER_AUTH_REQUIRED", and assert
ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFuncInvoked is false so expiry
behavior is covered separately from malformed-token handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 23117b52-d409-4e23-9990-03eb9a0b5c7e
📒 Files selected for processing (1)
server/service/orbit_eua_test.go
getvictor
left a comment
getvictor
left a comment
There was a problem hiding this comment.
Looks good overall. Left a few comments.
| NewSTSAuthTokenWithDeviceID(upn string, deviceID string) (string, error) | ||
|
|
||
| // GetSTSAuthTokenUPNClaim validates the given token and returns the UPN claim | ||
| GetSTSAuthTokenUPNClaim(token string) (string, error) | ||
|
|
||
| // GetSTSAuthTokenClaims validates the given token and returns the UPN and device ID claims. | ||
| GetSTSAuthTokenClaims(token string) (upn string, deviceID string, err error) |
There was a problem hiding this comment.
For the new methods, using STS Auth feels misleading since this is not part of the STS flow. I'd prefer just EUAToken
| } | ||
|
|
||
| // GetSTSAuthTokenClaims validates the given token and returns the UPN and device ID claims. | ||
| func (m *manager) GetSTSAuthTokenClaims(tokenStr string) (string, string, error) { |
There was a problem hiding this comment.
Nit. Feels simpler to return the whole claims struct and not the separate strings.
There was a problem hiding this comment.
I realized the use of STS is mixed in with the EUA which is very confusing. So I created a new type EUATokenClaims.
| } | ||
| device, err := svc.ds.MDMWindowsGetEnrolledDeviceWithDeviceID(ctx, deviceID) | ||
| if err != nil { | ||
| svc.logger.WarnContext(ctx, "unable to fetch windows mdm enrollment for EUA token generation", "err", err, "device_id", deviceID) |
There was a problem hiding this comment.
Why not ErrorContext? This only happens when there is a DB/connection issue, right? Same for the error below.
| </Validation> | ||
| <Enforcement> | ||
| <CommandLine>/quiet FLEET_URL="` + fleetURL + `" FLEET_SECRET="` + globalEnrollSecret + `" ENABLE_SCRIPTS="True"</CommandLine> | ||
| <CommandLine>/quiet FLEET_URL="` + fleetURL + `" FLEET_SECRET="` + globalEnrollSecret + `" ENABLE_SCRIPTS="True"` + euaTokenArg + `</CommandLine> |
There was a problem hiding this comment.
Nit. Is there an integration test somewhere where we can test this?
| return "", "", ctxerr.Wrap(ctx, err, "inserting mdm idp account for EUA token") | ||
| } | ||
| // Re-fetch to get the UUID assigned by the DB. | ||
| acct, err = svc.ds.GetMDMIdPAccountByEmail(ctx, upn) |
There was a problem hiding this comment.
This is the problematic read-after-write pattern that fails due to DB replica latency. For the read context, you can use ctxdb.RequirePrimary(ctx, true),
|
|
||
| if updated { | ||
| scimUser, err := svc.ds.ScimUserByUserNameOrEmail(ctx, euaUPN, euaUPN) | ||
| //nolint:gocritic // ignore ifElseChain |
There was a problem hiding this comment.
Why not just use switch here?
There was a problem hiding this comment.
It looked weird with the different conditions. But can switch back (pun intended! lol)
| svc := newTestServiceWithWSTEP(t, ds) | ||
|
|
||
| ds.MDMWindowsGetEnrolledDeviceWithDeviceIDFunc = func(ctx context.Context, mdmDeviceID string) (*fleet.MDMWindowsEnrolledDevice, error) { | ||
| return nil, nil |
There was a problem hiding this comment.
Nit. Should this return notFoundError?
| require.True(t, ds.AssociateHostMDMIdPAccountDBFuncInvoked, "should still link idp account even when enrollment has host_uuid") | ||
| }) | ||
|
|
||
| t.Run("expired or invalid token falls back to END_USER_AUTH_REQUIRED", func(t *testing.T) { |
There was a problem hiding this comment.
Nit. This doesn't actually test an expired token.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/service/integration_mdm_test.go (1)
9188-9190: Strengthen assertions to validate non-empty argument values.On Line 9188-Line 9190,
require.Containscan still pass for empty values (for example,EUA_TOKEN=""). Prefer regex assertions that require quoted non-empty content.Suggested test hardening
- require.Contains(t, installJob.Product.CommandLine, `EUA_TOKEN="`) - require.Contains(t, installJob.Product.CommandLine, `FLEET_URL="`) - require.Contains(t, installJob.Product.CommandLine, `FLEET_SECRET="`) + require.Regexp(t, `\bEUA_TOKEN="[^"]+"`, installJob.Product.CommandLine) + require.Regexp(t, `\bFLEET_URL="[^"]+"`, installJob.Product.CommandLine) + require.Regexp(t, `\bFLEET_SECRET="[^"]+"`, installJob.Product.CommandLine)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/service/integration_mdm_test.go` around lines 9188 - 9190, The current assertions check for presence of keys in installJob.Product.CommandLine but allow empty quoted values; update the test to assert non-empty quoted values using regex. Replace the three require.Contains checks with require.Regexp validations on installJob.Product.CommandLine that match patterns like EUA_TOKEN="[^"]+", FLEET_URL="[^"]+" and FLEET_SECRET="[^"]+" so each captured value inside quotes is at least one character; keep the same installJob and Product.CommandLine identifiers to locate where to change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/mdm/microsoft/wstep.go`:
- Around line 280-282: GetEUATokenClaims (and similarly GetSTSTokenClaims) uses
jwt.ParseWithClaims with a keyfunc that returns m.identityCert.PublicKey but
does not validate the token's alg, allowing algorithm confusion attacks; update
the keyfunc passed to jwt.ParseWithClaims to assert/verify the token.Method is
the expected RSA signing method (e.g., jwt.SigningMethodRS256 or check
token.Method.Alg() == jwt.SigningMethodRS256.Alg()) before returning
m.identityCert.PublicKey, and return an error if the algorithm is not RS256 so
untrusted alg values are rejected.
---
Nitpick comments:
In `@server/service/integration_mdm_test.go`:
- Around line 9188-9190: The current assertions check for presence of keys in
installJob.Product.CommandLine but allow empty quoted values; update the test to
assert non-empty quoted values using regex. Replace the three require.Contains
checks with require.Regexp validations on installJob.Product.CommandLine that
match patterns like EUA_TOKEN="[^"]+", FLEET_URL="[^"]+" and
FLEET_SECRET="[^"]+" so each captured value inside quotes is at least one
character; keep the same installJob and Product.CommandLine identifiers to
locate where to change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1eeb851b-0387-4f67-8ab7-034b73c0d07b
📒 Files selected for processing (6)
server/mdm/microsoft/wstep.goserver/mdm/microsoft/wstep_test.goserver/service/integration_mdm_test.goserver/service/microsoft_mdm.goserver/service/orbit.goserver/service/orbit_eua_test.go
✅ Files skipped from review due to trivial changes (2)
- server/mdm/microsoft/wstep_test.go
- server/service/orbit_eua_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- server/service/microsoft_mdm.go
getvictor
left a comment
getvictor
left a comment
There was a problem hiding this comment.
Looks good. Please take a look at the vulnerability that CodeRabbit found to see if it is valid.
|
@getvictor The vulnerability code rabbit mentioned is not really a concern. The |
ksykulev
force-pushed
the
41381-eua-ms-installer
branch
from
2d7853c to
264fe5b
Compare
ksykulev
requested review from
getvictor
and removed request for
a team,
allenhouchins,
ddribeiro,
eashaw,
georgekarrv,
ireedy,
lucasmrod,
lukeheath,
mike-j-thomas,
rachaelshaw,
sgress454 and
sharon-fdm
Related to a vulnerability found when working on #43295 #43295 (comment) `golang-jwt/jwt/v5` library already mitigates this, however, we are using `v4` which does not include this check. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Enforced RSA-only validation for JWTs used in authentication; tokens signed with non-RSA algorithms are now rejected. * **Tests** * Added tests to verify that non-RSA and unsigned JWTs are rejected and produce the expected error. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Related issue: Resolves #41381
Checklist for submitter
If some of the following don't apply, delete the relevant line.
Changes file added for user-visible changes in
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Input data is properly validated,
SELECT *is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.Testing
Summary by CodeRabbit
New Features
Tests
Documentation