-
Notifications
You must be signed in to change notification settings - Fork 347
feat(api): credential-backed auto re-login for browser automations #3410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -206,6 +206,24 @@ export class BrowserAuthProfileService { | |||||
| return { profile: updated, auth }; | ||||||
| } | ||||||
|
|
||||||
| async markVerified(input: { organizationId: string; profileId: string }) { | ||||||
| const profile = await this.getProfile(input); | ||||||
| if (!profile) { | ||||||
| throw new NotFoundException('Browser auth profile not found'); | ||||||
| } | ||||||
| // Avoid a needless write when the profile is already verified. | ||||||
| if (profile.status === 'verified') return profile; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Successful runs for an already-verified profile leave Prompt for AI agents
Suggested change
|
||||||
|
|
||||||
| return db.browserAuthProfile.update({ | ||||||
| where: { id: profile.id }, | ||||||
| data: { | ||||||
| status: 'verified', | ||||||
| lastVerifiedAt: new Date(), | ||||||
| blockedReason: null, | ||||||
| }, | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| async markNeedsReauth(input: { | ||||||
| organizationId: string; | ||||||
| profileId: string; | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ import { | |||||
| ResolveAuthProfileDto, | ||||||
| ResolveAuthProfileResponseDto, | ||||||
| SessionResponseDto, | ||||||
| StoreAuthProfileCredentialsDto, | ||||||
| VerifyAuthProfileResponseDto, | ||||||
| VerifyAuthProfileSessionDto, | ||||||
| } from './dto/browserbase.dto'; | ||||||
|
|
@@ -112,6 +113,30 @@ export class BrowserAuthProfilesController { | |||||
| })) as VerifyAuthProfileResponseDto; | ||||||
| } | ||||||
|
|
||||||
| @Post('profiles/:profileId/credentials') | ||||||
| @RequirePermission('integration', 'update') | ||||||
| @ApiOperation({ | ||||||
| summary: 'Store browser auth profile credentials', | ||||||
| description: | ||||||
| 'Store the login (username, password, optional authenticator setup key) for a browser auth profile in the organization vault so scheduled and manual runs can sign in automatically when a session expires.', | ||||||
| }) | ||||||
| @ApiParam({ name: 'profileId', description: 'Browser auth profile ID' }) | ||||||
| @ApiBody({ type: StoreAuthProfileCredentialsDto }) | ||||||
| @ApiResponse({ status: 200, type: BrowserAuthProfileResponseDto }) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Clients receive 201 from this route while the generated OpenAPI contract declares 200, so integrations that follow the documented response can mis-handle successful credential storage. The contract would be consistent if this documented 201 or the method explicitly returned 200. Prompt for AI agents
Suggested change
|
||||||
| async storeProfileCredentials( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The new credential endpoint has no controller coverage for RBAC protection, organization/profile forwarding, DTO mapping, or service errors, leaving the security-sensitive API boundary unverified. A focused controller spec would make these behaviors regression-tested. Prompt for AI agents |
||||||
| @OrganizationId() organizationId: string, | ||||||
| @Param('profileId') profileId: string, | ||||||
| @Body() dto: StoreAuthProfileCredentialsDto, | ||||||
| ): Promise<BrowserAuthProfileResponseDto> { | ||||||
| return (await this.browserbaseService.storeAuthProfileCredentials({ | ||||||
| organizationId, | ||||||
| profileId, | ||||||
| username: dto.username, | ||||||
| password: dto.password, | ||||||
| totpSeed: dto.totpSeed, | ||||||
| })) as BrowserAuthProfileResponseDto; | ||||||
| } | ||||||
|
|
||||||
| @Post('profiles/:profileId/needs-reauth') | ||||||
| @RequirePermission('integration', 'update') | ||||||
| @ApiOperation({ | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -137,7 +137,12 @@ export class BrowserAutomationExecutionService { | |||||||
| profileId: profile.id, | ||||||||
| }); | ||||||||
|
|
||||||||
| if (profile.status !== 'verified') { | ||||||||
| // A profile with stored credentials can recover an expired session on its | ||||||||
| // own, so let a needs_reauth profile attempt the run instead of blocking it. | ||||||||
| // Profiles that are blocked or never verified still require a human. | ||||||||
| const canAutoRelogin = | ||||||||
| profile.status === 'needs_reauth' && Boolean(profile.vaultProvider); | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Profiles with only Prompt for AI agents
Suggested change
|
||||||||
| if (profile.status !== 'verified' && !canAutoRelogin) { | ||||||||
| const result = this.profileBlockedResult(profile.status); | ||||||||
| await this.runs.finishRun({ | ||||||||
| runId: run.id, | ||||||||
|
|
@@ -188,6 +193,16 @@ export class BrowserAutomationExecutionService { | |||||||
| profileId: string; | ||||||||
| result: BrowserEvidenceRunResult; | ||||||||
| }) { | ||||||||
| // A successful run proves the session is valid again — clear any prior | ||||||||
| // needs_reauth/blocked state (e.g. after a credential-backed auto sign-in). | ||||||||
| if (input.result.success) { | ||||||||
| await this.profiles.markVerified({ | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: A successful live-session execution can now mark the run's profile verified even when the supplied Browserbase session is not that profile's session. The profile/session ownership check should occur before execution, otherwise an unrelated session can clear Prompt for AI agents |
||||||||
| organizationId: input.organizationId, | ||||||||
| profileId: input.profileId, | ||||||||
| }); | ||||||||
| return; | ||||||||
| } | ||||||||
|
|
||||||||
| if (input.result.failureCode === 'needs_reauth') { | ||||||||
| await this.profiles.markNeedsReauth({ | ||||||||
| organizationId: input.organizationId, | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| import { | ||
| performCredentialLogin, | ||
| reloginWithStoredCredentials, | ||
| } from './browser-credential-login'; | ||
| import type { BrowserCredentialVaultAdapter } from './credential-vault'; | ||
| import type { BrowserbaseSessionService } from './browserbase-session.service'; | ||
| import type { BrowserEvidenceSessionInput } from './browser-evidence-runner.service'; | ||
|
|
||
| type Stagehand = import('@browserbasehq/stagehand').Stagehand; | ||
|
|
||
| const makeStagehand = () => ({ act: jest.fn().mockResolvedValue(undefined) }); | ||
| const makePage = () => ({ goto: jest.fn().mockResolvedValue(undefined) }); | ||
| const makeSessions = (page: ReturnType<typeof makePage>) => ({ | ||
| ensureActivePage: jest.fn().mockResolvedValue(page), | ||
| }); | ||
|
|
||
| const baseInput: BrowserEvidenceSessionInput = { | ||
| organizationId: 'org_1', | ||
| automationId: 'bau_1', | ||
| runId: 'bar_1', | ||
| targetUrl: 'https://vendor.example.com/app', | ||
| instruction: 'capture evidence', | ||
| profile: { | ||
| id: 'bap_1', | ||
| hostname: 'vendor.example.com', | ||
| contextId: 'ctx_1', | ||
| vaultProvider: '1password', | ||
| vaultExternalItemRef: 'op://v/i', | ||
| vaultConnectionId: 'v', | ||
| }, | ||
| sessionId: 'sess_1', | ||
| }; | ||
|
|
||
| describe('performCredentialLogin', () => { | ||
| beforeEach(() => jest.useFakeTimers()); | ||
| afterEach(() => jest.useRealTimers()); | ||
|
|
||
| it('passes secrets through act variables and never in the prompt text', async () => { | ||
| const stagehand = makeStagehand(); | ||
| const password = 'sup3r-s3cret-passphrase'; | ||
|
|
||
| const promise = performCredentialLogin({ | ||
| stagehand: stagehand as unknown as Stagehand, | ||
| credentials: { username: 'alice', password, totpCode: '424242' }, | ||
| log: jest.fn(), | ||
| }); | ||
| await jest.runAllTimersAsync(); | ||
| await promise; | ||
|
|
||
| expect(stagehand.act).toHaveBeenNthCalledWith( | ||
| 1, | ||
| expect.stringContaining('%username%'), | ||
| { variables: { username: 'alice', password } }, | ||
| ); | ||
| expect(stagehand.act).toHaveBeenNthCalledWith( | ||
| 2, | ||
| expect.stringContaining('%code%'), | ||
| { variables: { code: '424242' } }, | ||
| ); | ||
|
|
||
| // The actual secret values must not appear in the instruction sent to the LLM. | ||
| const credentialInstruction = stagehand.act.mock.calls[0][0] as string; | ||
| expect(credentialInstruction).not.toContain(password); | ||
| expect(credentialInstruction).not.toContain('alice'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('reloginWithStoredCredentials', () => { | ||
| beforeEach(() => jest.useFakeTimers()); | ||
| afterEach(() => jest.useRealTimers()); | ||
|
|
||
| const runRelogin = async (args: { | ||
| stagehand: ReturnType<typeof makeStagehand>; | ||
| sessions: ReturnType<typeof makeSessions>; | ||
| vault: BrowserCredentialVaultAdapter; | ||
| verifyLoggedIn: jest.Mock; | ||
| }) => { | ||
| const promise = reloginWithStoredCredentials({ | ||
| stagehand: args.stagehand as unknown as Stagehand, | ||
| sessions: args.sessions as unknown as BrowserbaseSessionService, | ||
| vault: args.vault, | ||
| input: baseInput, | ||
| verifyLoggedIn: args.verifyLoggedIn, | ||
| log: jest.fn(), | ||
| }); | ||
| await jest.runAllTimersAsync(); | ||
| return promise; | ||
| }; | ||
|
|
||
| it('does not attempt a login when no credentials are stored', async () => { | ||
| const stagehand = makeStagehand(); | ||
| const page = makePage(); | ||
| const vault: BrowserCredentialVaultAdapter = { | ||
| resolveCredentialReference: jest.fn().mockResolvedValue(null), | ||
| }; | ||
|
|
||
| const result = await runRelogin({ | ||
| stagehand, | ||
| sessions: makeSessions(page), | ||
| vault, | ||
| verifyLoggedIn: jest.fn().mockResolvedValue(false), | ||
| }); | ||
|
|
||
| expect(result.isLoggedIn).toBe(false); | ||
| expect(result.reason).toMatch(/no stored credentials/i); | ||
| expect(stagehand.act).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('signs in and returns to the target URL when verification passes', async () => { | ||
| const stagehand = makeStagehand(); | ||
| const page = makePage(); | ||
| const vault: BrowserCredentialVaultAdapter = { | ||
| resolveCredentialReference: jest | ||
| .fn() | ||
| .mockResolvedValue({ username: 'alice', password: 'pw' }), | ||
| }; | ||
|
|
||
| const result = await runRelogin({ | ||
| stagehand, | ||
| sessions: makeSessions(page), | ||
| vault, | ||
| verifyLoggedIn: jest.fn().mockResolvedValue(true), | ||
| }); | ||
|
|
||
| expect(result.isLoggedIn).toBe(true); | ||
| expect(stagehand.act).toHaveBeenCalledTimes(1); | ||
| expect(page.goto).toHaveBeenCalledWith( | ||
| baseInput.targetUrl, | ||
| expect.objectContaining({ waitUntil: 'domcontentloaded' }), | ||
| ); | ||
| }); | ||
|
|
||
| it('retries once with a freshly resolved TOTP code', async () => { | ||
| const stagehand = makeStagehand(); | ||
| const page = makePage(); | ||
| const resolveCredentialReference = jest | ||
| .fn() | ||
| .mockResolvedValueOnce({ | ||
| username: 'alice', | ||
| password: 'pw', | ||
| totpCode: '111111', | ||
| }) | ||
| .mockResolvedValueOnce({ | ||
| username: 'alice', | ||
| password: 'pw', | ||
| totpCode: '222222', | ||
| }); | ||
| const vault: BrowserCredentialVaultAdapter = { resolveCredentialReference }; | ||
| const verifyLoggedIn = jest | ||
| .fn() | ||
| .mockResolvedValueOnce(false) | ||
| .mockResolvedValueOnce(true); | ||
|
|
||
| const result = await runRelogin({ | ||
| stagehand, | ||
| sessions: makeSessions(page), | ||
| vault, | ||
| verifyLoggedIn, | ||
| }); | ||
|
|
||
| expect(result.isLoggedIn).toBe(true); | ||
| expect(resolveCredentialReference).toHaveBeenCalledTimes(2); | ||
| // Two login attempts: each enters credentials + a TOTP code (2 acts each). | ||
| expect(stagehand.act).toHaveBeenCalledTimes(4); | ||
| }); | ||
|
|
||
| it('gives up (user action required) when sign-in never authenticates', async () => { | ||
| const stagehand = makeStagehand(); | ||
| const page = makePage(); | ||
| const vault: BrowserCredentialVaultAdapter = { | ||
| resolveCredentialReference: jest | ||
| .fn() | ||
| .mockResolvedValue({ username: 'alice', password: 'pw' }), | ||
| }; | ||
|
|
||
| const result = await runRelogin({ | ||
| stagehand, | ||
| sessions: makeSessions(page), | ||
| vault, | ||
| verifyLoggedIn: jest.fn().mockResolvedValue(false), | ||
| }); | ||
|
|
||
| expect(result.isLoggedIn).toBe(false); | ||
| expect(result.reason).toMatch(/user action/i); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: The new successful-run profile transition is currently untested, so regressions in status,
lastVerifiedAt, or blocked-reason clearing can pass the API suite unnoticed. Adding service tests for an unverified/reauth profile, an already-verified profile, and a missing profile would cover this behavior.Prompt for AI agents