Skip to content

feat: add offline credentials endpoints - #1819

Merged
DmytroZaichenkoDev merged 4 commits into
developmentfrom
feat/offline-credentials-endpoints
Aug 12, 2026
Merged

feat: add offline credentials endpoints#1819
DmytroZaichenkoDev merged 4 commits into
developmentfrom
feat/offline-credentials-endpoints

Conversation

@DmytroZaichenkoDev

Copy link
Copy Markdown
Contributor

Part 2/4 of the offline-delegation split (replaces #1815). Based on #1818.

Adds /v1/user/offline-credentials (status), /signin and /signout: a user grants DIAL one platform-wide offline credential, stored in their own bucket under a reserved id no app-scoped endpoint can address. Sign-in exchanges the code via the provider's offlineClient, verifies through the ID token that the code belongs to the caller, and records the issuer for later refresh. Refresh responses without a refresh token keep the existing one (RFC 6749 §6). All operations audited; shared error mapping extracted into ExternalServiceErrors.

🤖 Generated with Claude Code

@ai-dial-actions

This comment has been minimized.

@astsiapanay astsiapanay left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated code review (max effort: 10 finder angles + 1-vote verification + gap sweep). 12 verified findings posted inline below, ranked by severity.

Two additional verified findings from the same review are not included here because they belong to #1818 ("add DIAL_NATIVE auth type"), not this PR's diff:

  • DialNativeAuthSettingsValidator (new in #1818) duplicates NoneAuthSettingsValidator verbatim instead of reusing it.
  • ExternalServiceService.putExternalService validates new auth settings before an unconditional secret-preservation step reattaches the old client secret, with no re-check that the auth type didn't change — so converting a service to DIAL_NATIVE can silently leave its old OAuth secret persisted underneath a type whose entire premise is "no credential is declared." This is arguably the most severe finding of the whole review, but the touched file (ExternalServiceService.java) isn't part of either PR's diff, so it has no line to anchor a comment to here — flagging it in this summary instead.

@ai-dial-actions

This comment has been minimized.

astsiapanay
astsiapanay previously approved these changes Aug 12, 2026
@ai-dial-actions

This comment has been minimized.

Base automatically changed from feat/dial-native-auth-type to development August 12, 2026 13:00
Dmytro Zaichenko and others added 4 commits August 12, 2026 16:00
Adds /v1/user/offline-credentials (status), /signin and /signout: a user
grants DIAL one platform-wide offline credential — a refresh token stored in
their own bucket under a reserved id that no app-scoped endpoint can address.
Sign-in exchanges the authorization code via the identity provider's
offlineClient, verifies through the ID token that the code belongs to the
caller before anything is stored, and records the issuer so a later refresh
can resolve the provider without a caller token. Refresh responses that omit
a refresh token keep the existing one (RFC 6749 §6). All operations are
audited, and the shared external-service error mapping is extracted into
ExternalServiceErrors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A USER-level record stored while a service was OAUTH/API_KEY survives a
switch to DIAL_NATIVE — the type-change purge reaches only APPLICATION-level
records, and per-user buckets cannot be enumerated. The retrieval guard
treated only NONE as credential-less, so such a leftover was still served
with its old bearer token. DIAL_NATIVE now short-circuits both retrieval
paths the same way NONE does.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…token uniformly

A sign-in with an authorization code minted for another user returned 403
but was audited as a generic ERROR: the refusal threw a bare HttpException,
which the audit outcome mapping does not classify. It now throws
PermissionDeniedException — same 403, but the audit stream records DENIED,
so alerting on denied outcomes catches these attempts.

resolveProvider also promised to refuse a missing access token but returned
the provider unchecked when exactly one was configured; the check now runs
before the single-provider shortcut, matching the documented contract.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…face

- OfflineCredentialsStatus.of: the availability disjunct could never differ
  from plain 'connected' inside its guard; computed directly now.
- OfflineCredentialsController captures ResourceAuthSettingsService like its
  other dependencies instead of reaching through Proxy inline.
- The toolset half of the reservation test now derives the storage id from
  the real locator instead of asserting on a string literal.
- Dropped a matchesIssuer comment that restated the code.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@DmytroZaichenkoDev
DmytroZaichenkoDev force-pushed the feat/offline-credentials-endpoints branch from 9b6f6f3 to 0f301f6 Compare August 12, 2026 13:00
@ai-dial-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@DmytroZaichenkoDev
DmytroZaichenkoDev merged commit 97cb794 into development Aug 12, 2026
9 checks passed
@DmytroZaichenkoDev
DmytroZaichenkoDev deleted the feat/offline-credentials-endpoints branch August 12, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants