feat: add Google Domain-Wide Delegation (GOOGLE_DWD) auth support#153
Conversation
- Add GoogleDWDAuth message and regenerate connected_accounts_pb2.py
- Add google_dwd branch to CreateConnectedAccountRequest.to_proto()
- Add google_dwd branch to UpdateConnectedAccountRequest.to_proto()
- Switch ConnectedAccount.from_proto() to use WhichOneof("details")
- Add 5 unit tests for request/response serialization and oauth_token isolation
|
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:
WalkthroughThis PR adds Google Domain-Wide Delegation (DWD) authorization support for connected accounts and extends the ConnectionClient with four new methods for managing environment and app-level connections. Request models validate and serialize ChangesGoogle DWD Authorization Serialization & Deserialization
Connection API Client Methods & Integration Tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scalekit/actions/models/requests/create_connected_account_request.py`:
- Around line 49-52: The branch that builds GoogleDWDAuth must validate that
google_dwd.subject is provided instead of defaulting to ""; in the block where
you read self.authorization_details["google_dwd"] (referencing
self.authorization_details, dwd_data, GoogleDWDAuth, AuthorizationDetails),
check that dwd_data.get("subject") is present and non-empty and raise a clear
exception (e.g., ValueError) if missing; only construct
GoogleDWDAuth(subject=...) and AuthorizationDetails(google_dwd=...) after the
subject validation succeeds.
In `@scalekit/actions/models/requests/update_connected_account_request.py`:
- Around line 50-53: The branch handling
self.authorization_details["google_dwd"] currently defaults
GoogleDWDAuth(subject="") which allows blank subjects; change this to validate
and fail fast: in the update logic where authorization_details and "google_dwd"
are checked, extract dwd_data and assert that dwd_data.get("subject") exists and
is non-empty (e.g., raise ValueError or a RequestValidationError) before
constructing GoogleDWDAuth(subject=...), and only then build
AuthorizationDetails(google_dwd=google_dwd); reference symbols:
self.authorization_details, "google_dwd", dwd_data, GoogleDWDAuth,
AuthorizationDetails.
In `@sdk.py`:
- Around line 65-73: Update the docblock to remove the stale claim that
GOOGLE_DWD is absent from ConnectorType and instead state that the regenerated
proto includes ConnectorType.GOOGLE_DWD; clarify that callers can rely on
ConnectorType (e.g., ConnectorType.GOOGLE_DWD) rather than the prior workaround,
and reflect the current serializer behavior that create/update requests only
write subject (not access_token/scopes/token_expires_at). Keep the existing
guidance about using WhichOneof("details") in from_proto() where appropriate and
retaining the token_expires_at HasField("token_expires_at") guard before calling
ToDatetime() to avoid AttributeError.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46a70b52-3497-49b4-bf57-aa21b5c66e1d
📒 Files selected for processing (8)
scalekit/actions/models/requests/create_connected_account_request.pyscalekit/actions/models/requests/update_connected_account_request.pyscalekit/actions/models/responses/get_connected_account_auth_response.pyscalekit/v1/connected_accounts/connected_accounts_pb2.pyscalekit/v1/connected_accounts/connected_accounts_pb2.pyiscalekit/v1/connected_accounts/connected_accounts_pb2_grpc.pysdk.pytests/test_actions.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scalekit/actions/models/requests/create_connected_account_request.py`:
- Around line 49-55: The code assumes self.authorization_details["google_dwd"]
is a dict; add a type/shape guard for dwd_data before accessing .get so non-dict
values produce a clear validation error: check that self.authorization_details
and its "google_dwd" entry is a mapping (e.g., isinstance(dwd_data, dict)) and
if not raise ValueError("authorization_details.google_dwd must be an object");
then read subject, validate subject presence and raise the existing ValueError
if missing, and proceed to construct GoogleDWDAuth and AuthorizationDetails as
before (references: authorization_details, dwd_data, subject, GoogleDWDAuth,
AuthorizationDetails).
In `@scalekit/actions/models/requests/update_connected_account_request.py`:
- Around line 50-56: The code assumes authorization_details["google_dwd"] is a
dict and calls dwd_data.get(...), which can raise AttributeError for bad input;
in the update serialization branch (the elif handling authorization_details and
"google_dwd") validate that dwd_data is a dict (e.g., if not
isinstance(dwd_data, dict): raise ValueError("authorization_details.google_dwd
must be an object")), then proceed to extract subject and raise the existing
ValueError if missing, before constructing GoogleDWDAuth and
AuthorizationDetails; update the branch that sets google_dwd =
GoogleDWDAuth(subject=subject) / auth_details =
AuthorizationDetails(google_dwd=google_dwd) accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a96e0c5-555d-4366-b95e-536b353b90ee
📒 Files selected for processing (2)
scalekit/actions/models/requests/create_connected_account_request.pyscalekit/actions/models/requests/update_connected_account_request.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_actions.py (1)
784-860: ⚡ Quick winAdd a negative test for missing
google_dwd.subjectvalidation.The happy-path coverage is good, but Line 784–Line 860 doesn’t assert the new validation behavior when
subjectis absent. Add one failure-case test so the validation fix can’t regress silently.Proposed test addition
+ def test_google_dwd_create_request_missing_subject_raises(self): + """CreateConnectedAccountRequest rejects google_dwd without subject""" + from scalekit.actions.models.requests.create_connected_account_request import CreateConnectedAccountRequest + + req = CreateConnectedAccountRequest( + connection_name="GDWD", + identifier="admin@example.com", + authorization_details={"google_dwd": {}} + ) + + with self.assertRaises(ValueError) as context: + req.to_proto() + self.assertIn("subject", str(context.exception).lower())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_actions.py` around lines 784 - 860, Add a negative test that ensures missing google_dwd.subject triggers validation: create a test (e.g. test_google_dwd_create_request_missing_subject_raises) that constructs CreateConnectedAccountRequest with authorization_details={"google_dwd": {}} and assert that calling req.to_proto() raises an exception (use assertRaises(ValueError) or the specific validation exception used in your codebase). Reference CreateConnectedAccountRequest and to_proto() so the test fails if the new validation is removed or bypassed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_actions.py`:
- Around line 784-860: Add a negative test that ensures missing
google_dwd.subject triggers validation: create a test (e.g.
test_google_dwd_create_request_missing_subject_raises) that constructs
CreateConnectedAccountRequest with authorization_details={"google_dwd": {}} and
assert that calling req.to_proto() raises an exception (use
assertRaises(ValueError) or the specific validation exception used in your
codebase). Reference CreateConnectedAccountRequest and to_proto() so the test
fails if the new validation is removed or bypassed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 06f5fd51-12af-441e-bee6-40e60c2df6c1
📒 Files selected for processing (1)
tests/test_actions.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scalekit/actions/actions.py`:
- Around line 857-961: These connection methods assume self._connection_client
exists and will raise an unhelpful AttributeError if not; add a guard at the
start of list_connections, get_connection, create_connection, and
update_connection to check if self._connection_client is truthy and, if not,
raise a clear error (e.g., ValueError or a specific SDK exception) with a
message like "connection client is not configured; initialize ActionClient with
a connection_client" so callers get a descriptive failure instead of an
AttributeError.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae380591-a805-4f81-bf0c-a9cff6702c96
📒 Files selected for processing (4)
Makefilescalekit/actions/actions.pyscalekit/connection.pytests/test_connection.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/test_connection.py (1)
163-184: ⚡ Quick winAssert the
client_secretupdate tooThis test updates
client_secretbut never verifies it. Add an assertion (or redaction-safe equivalent) so the update path is fully covered.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_connection.py` around lines 163 - 184, The test updates the OAuth client_secret but doesn't assert it; update the test in tests/test_connection.py (the block creating UpdateConnection and OAuthConnectionConfig) to also verify the client_secret change by asserting conn.oauth_config.client_secret is updated — use a redaction-safe check if values are masked (e.g., compare presence/length or a masked placeholder) so the update path for UpdateConnection and OAuthConnectionConfig.client_secret is fully covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_connection.py`:
- Line 205: The test currently embeds a PEM-looking private key in the
fake_sa_json (variable fake_sa_json), which triggers secret scanners; replace
the PEM-formatted value with a non-PEM placeholder (e.g.,
"private_key":"REDACTED_PRIVATE_KEY" or a random base64 string that does not
include "BEGIN" / "END" headers) and, if the test asserts parsing of a key
format, adjust the assertion/mocks in the test to accept the placeholder value
so behavior remains the same; update only the fake_sa_json string and any
dependent expectations in the test (refer to fake_sa_json) to remove any
real/PEM headers and avoid secret-scanner triggers.
- Around line 95-232: Tests create environment/app connections in methods like
test_create_environment_connection_with_flags,
test_list_app_connections_with_provider_filter, test_get_custom_connection,
test_update_environment_connection, and test_google_dwd_create_update_get but do
not unregister them; update the test class to track created connection IDs
(e.g., add self._created_connection_ids initialized in setUp) and after each
create_environment_connection call append the returned connection.id (from
create_response[0].connection.id) to that list; modify tearDown to iterate over
self._created_connection_ids and call the API to delete each environment
connection (e.g.,
self.scalekit_client.connection.delete_environment_connection(connection_id=...))
swallowing/not failing on 404/errors so teardown is robust, leaving the existing
self.conn_id deletion intact.
---
Nitpick comments:
In `@tests/test_connection.py`:
- Around line 163-184: The test updates the OAuth client_secret but doesn't
assert it; update the test in tests/test_connection.py (the block creating
UpdateConnection and OAuthConnectionConfig) to also verify the client_secret
change by asserting conn.oauth_config.client_secret is updated — use a
redaction-safe check if values are masked (e.g., compare presence/length or a
masked placeholder) so the update path for UpdateConnection and
OAuthConnectionConfig.client_secret is fully covered.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8aa391cb-46f3-4c64-994a-e1f94fadb0dc
📒 Files selected for processing (3)
scalekit/actions/models/requests/create_connected_account_request.pyscalekit/actions/models/requests/update_connected_account_request.pytests/test_connection.py
🚧 Files skipped from review as they are similar to previous changes (2)
- scalekit/actions/models/requests/update_connected_account_request.py
- scalekit/actions/models/requests/create_connected_account_request.py
Summary
GoogleDWDAuthmessage to proto and regenerateconnected_accounts_pb2.pygoogle_dwdbranch toCreateConnectedAccountRequest.to_proto()andUpdateConnectedAccountRequest.to_proto()ConnectedAccount.from_proto()to useWhichOneof("details")for correct deserialization of all auth types includinggoogle_dwdTest plan
python3 -m pytest tests/test_actions.py -k "google_dwd" -v— 5 tests passauthorization_details={"google_dwd": {"subject": "user@domain.com"}}authorization_details.google_dwdcontainssubject,access_token,scopes,token_expires_at🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests