Action-revoke-user-sessions#67
Open
cvkramer wants to merge 1 commit into
Open
Conversation
Implement action to revoke all active user sessions (web and device) using Google Admin Directory API. Includes comprehensive tests, and handler implementation. Changes: - Add revokeUserSessionsHandler in pkg/connector/actions.go - Define revokeUserSessionsActionSchema in pkg/connector/connector.go - Add comprehensive test suite (4 test cases covering success, validation, error handling, and non-idempotent behavior) Technical details: - Requires admin.directory.user.security OAuth scope - Uses ACTION_TYPE_ACCOUNT for UI categorization - Not idempotent (Google API has no session state check) - Input validation: required user_email with format checking - Returns success status and user email on completion
WalkthroughA new "revoke user sessions" action is added to the Google Workspace connector: it validates a provided user_email, calls the Directory service SignOut API for that user, and returns a success response; tests and action schema registration are included. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as revokeUserSessionsHandler
participant Service as DirectoryService
participant API as GoogleAPI_SignOut
Client->>Handler: invoke action with user_email
Handler->>Handler: validate user_email
Handler->>Service: Get admin Directory service (AdminDirectoryUserSecurityScope)
Service-->>Handler: directory service instance
Handler->>API: POST /users/{userKey}/signOut
API-->>Handler: 204 No Content / error
Handler->>Client: return { success: true, user_email } / error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In `@pkg/connector/actions_test.go`:
- Around line 46-72: Run goimports (or gofmt) on the test file to fix
formatting/import ordering issues flagged by static analysis; format the file
containing the mux.HandleFunc(...) test handler (the function manipulating
state.mtx, state.signOutCount and setting u.SignedOut and writing
http.StatusNoContent) so imports are organized and the file passes
goimports/gofmt checks, then commit the formatted file.
Comment on lines
46
to
+72
| mux.HandleFunc("/admin/directory/v1/users/", func(w http.ResponseWriter, r *http.Request) { | ||
| // path suffix is userKey | ||
| userKey := strings.TrimPrefix(r.URL.Path, "/admin/directory/v1/users/") | ||
| // path suffix is userKey, may have /signOut suffix | ||
| path := strings.TrimPrefix(r.URL.Path, "/admin/directory/v1/users/") | ||
|
|
||
| // Check for signOut endpoint | ||
| if strings.HasSuffix(path, "/signOut") { | ||
| userKey := strings.TrimSuffix(path, "/signOut") | ||
| state.mtx.Lock() | ||
| defer state.mtx.Unlock() | ||
|
|
||
| if r.Method == http.MethodPost { | ||
| state.signOutCount++ | ||
| u := state.users[userKey] | ||
| if u == nil { | ||
| http.Error(w, "not found", http.StatusNotFound) | ||
| return | ||
| } | ||
| u.SignedOut = true | ||
| w.WriteHeader(http.StatusNoContent) // SignOut returns 204 No Content | ||
| return | ||
| } | ||
| http.Error(w, "method not allowed", http.StatusMethodNotAllowed) | ||
| return | ||
| } | ||
|
|
||
| // Regular user operations | ||
| userKey := path |
There was a problem hiding this comment.
Fix formatting issue flagged by static analysis.
The static analysis tool (goimports) indicates this file is not properly formatted. Please run goimports or gofmt to fix the formatting.
🔧 Suggested fix
Run the following command to fix:
goimports -w pkg/connector/actions_test.goOr if using gofmt:
gofmt -w pkg/connector/actions_test.go🧰 Tools
🪛 GitHub Check: go-lint
[failure] 49-49:
File is not properly formatted (goimports)
🤖 Prompt for AI Agents
In `@pkg/connector/actions_test.go` around lines 46 - 72, Run goimports (or gofmt)
on the test file to fix formatting/import ordering issues flagged by static
analysis; format the file containing the mux.HandleFunc(...) test handler (the
function manipulating state.mtx, state.signOutCount and setting u.SignedOut and
writing http.StatusNoContent) so imports are organized and the file passes
goimports/gofmt checks, then commit the formatted file.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pkg/connector/actions_test.go`:
- Around line 421-429: The mutex (state.mtx) is locked then t.Fatalf is called
on failure which can leave the mutex locked; change the pattern in the
verification blocks (where state.mtx.Lock() is used before checking
state.signOutCount and state.users["alice@example.com"].SignedOut, and the
similar block around sign-in checks) to immediately defer state.mtx.Unlock()
after acquiring the lock so the mutex is always released even if t.Fatalf exits
the goroutine; keep the same assertions (state.signOutCount and state.users[...]
.SignedOut) but ensure the defer is placed right after calling state.mtx.Lock().
Comment on lines
+421
to
+429
| // Verify API was called | ||
| state.mtx.Lock() | ||
| if state.signOutCount != 1 { | ||
| t.Fatalf("expected one signOut call, got: %d", state.signOutCount) | ||
| } | ||
| if !state.users["alice@example.com"].SignedOut { | ||
| t.Fatalf("expected user to be signed out") | ||
| } | ||
| state.mtx.Unlock() |
There was a problem hiding this comment.
Avoid leaving the mutex locked on test failure.
Fatalf exits the goroutine; without a deferred unlock, the lock can remain held and cause test teardown hangs.
🔧 Suggested fix
// Verify API was called
state.mtx.Lock()
+ defer state.mtx.Unlock()
if state.signOutCount != 1 {
t.Fatalf("expected one signOut call, got: %d", state.signOutCount)
}
if !state.users["alice@example.com"].SignedOut {
t.Fatalf("expected user to be signed out")
}
- state.mtx.Unlock()
@@
- state.mtx.Lock()
- if state.signOutCount != firstCallCount+1 {
- t.Fatalf("expected another revoke call (not idempotent), got %d total calls", state.signOutCount)
- }
- state.mtx.Unlock()
+ func() {
+ state.mtx.Lock()
+ defer state.mtx.Unlock()
+ if state.signOutCount != firstCallCount+1 {
+ t.Fatalf("expected another revoke call (not idempotent), got %d total calls", state.signOutCount)
+ }
+ }()Also applies to: 534-538
🤖 Prompt for AI Agents
In `@pkg/connector/actions_test.go` around lines 421 - 429, The mutex (state.mtx)
is locked then t.Fatalf is called on failure which can leave the mutex locked;
change the pattern in the verification blocks (where state.mtx.Lock() is used
before checking state.signOutCount and
state.users["alice@example.com"].SignedOut, and the similar block around sign-in
checks) to immediately defer state.mtx.Unlock() after acquiring the lock so the
mutex is always released even if t.Fatalf exits the goroutine; keep the same
assertions (state.signOutCount and state.users[...] .SignedOut) but ensure the
defer is placed right after calling state.mtx.Lock().
cvkramer
force-pushed
the
action-revoke-user-sessions
branch
from
b8ba591 to
4f0e9b9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement action to revoke all active user sessions (web and device)
using Google Admin Directory API. Includes comprehensive tests,
and handler implementation.
Changes:
error handling, and non-idempotent behavior)
Technical details:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.