Skip to content

fix: return login statuses on platform toolset/application GET #1928 - #1960

Open
KirylKurnosenka wants to merge 3 commits into
developmentfrom
fix/issue-1928
Open

KirylKurnosenka wants to merge 3 commits into
developmentfrom
fix/issue-1928

Conversation

@KirylKurnosenka

@KirylKurnosenka KirylKurnosenka commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

GET v1/{toolsets|applications}/platform/{name} returned entities without the computed login statuses (global_auth_status, user_level_auth_status, app_level_auth_status), so the Admin UI showed the toolset/application as logged out even after a successful sign-in. The blobs never persist statuses (the write path strips computed fields), so the platform-bucket GET must compute them per caller — this adds that, reusing the same mechanisms the usage APIs already employ, and consolidates the status enrichment that was duplicated across controllers into one place.

Applicable issues

Description of changes

  • ConfigResourceController.handleGet case TOOL_SET: computes auth statuses via ResourceAuthStatusEnricher.enrichToolSet with the short-name credential scope (platform deployments are short-name keyed in the merged config — the same form /v1/ops/toolset/signin normalizes credentials under); per-call try/catch so a Redis hiccup degrades to omitted statuses instead of failing the GET
  • case APPLICATION: computes external-service statuses via ResourceAuthStatusEnricher.enrichApplication, using the short-name scope, before admin decrypt and secret redaction
  • statuses are set on the POJO before serialization; secret redaction is unchanged; the write path already strips echoed statuses, so nothing new can be persisted
  • consolidated the three duplicated external-service status enrichment loops (ApplicationController, ResourceController, ConfigResourceController) into a single ResourceAuthStatusEnricher.enrichApplication, which resolves scopes through the new CredentialsLocatorFactory.fromExternalService(appId, serviceId, context) — decoded-only contract, symmetric encoding of both parts, with the one mixed-form input (ApplicationController's id) normalized at its own boundary
  • per review: the enricher is renamed from ExternalServiceStatusEnricher and now also owns toolset status enrichment — enrichToolSet absorbs the id encoding from the controllers, which pass decoded ids; ToolSetService.setResourceAuthStatuses is removed and all callers (ConfigResourceController, ResourceController, ToolSetController) go through the enricher; ConfigResourceController takes ResourceAuthSettingsService via its constructor like its other services
  • new tests in PlatformAppToolsetApiTest: API_KEY toolset statuses on GET (sign-in flips both levels), application external-service statuses (USER + APPLICATION sign-in), and OAUTH toolset with client_secret_hint and status present together

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

🤖 Generated with Claude Code

Co-Authored-By: Claude Code <noreply@anthropic.com>
@KirylKurnosenka

KirylKurnosenka commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

/deploy-review

GitHub actions run: 34505549737

Stage Status
deploy-review Success ✅
chat Success ✅

@ai-dial-actions

This comment has been minimized.

case APPLICATION -> handleSingleGetFromBlob(ResourceTypes.APPLICATION,
(key, application) -> {
Application entity = (Application) application;
new ExternalServiceStatusEnricher(context, context.getProxy().getResourceAuthSettingsService())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

better add ResourceAuthSettingsService via constructor as we do with other services.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 6f146d8ResourceAuthSettingsService is now taken via the constructor alongside the other services (the controller was reworked to derive all of them from Proxy there).

* <p>Memoizes that lookup, so create one per response rather than per service.
*/
@Slf4j
public class ExternalServiceStatusEnricher {

@DmytroZaichenkoDev DmytroZaichenkoDev Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: re-name this to ResourceAuthStatusEnricher and move toolset status enrichment here as well.

public void enrichToolSet(String toolSetId, ToolSet toolSet) {                                     
     if (toolSet.getAuthSettings() == null) {                                                                                                                            
         return;       
      }                                                               
      resourceAuthSettingsService.setResourceAuthStatuses(CredentialsLocatorFactory.fromAnyUrl(
UrlUtil.encodePath(toolSetId), context, ResourceTypes.TOOL_SET),
      toolSet.getAuthSettings(), context.getInitiatorId());                                                                                                                  
  }

So we incapsulate this logic in enricher and use it for both branches in ConfigResourceController:

case APPLICATION -> ... authStatusEnricher.enrichApplication(path, entity.getExternalServices());          
case TOOL_SET    -> ... authStatusEnricher.enrichToolSet(path, entity);

It's also a good place to deal with encoded/decoded ids. Can we move this logic here from controller, where we do UrlUtil.encodePath(toolSetId)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in 6f146d8 — renamed to ResourceAuthStatusEnricher and moved toolset status enrichment here as enrichToolSet (also absorbing the UrlUtil.encodePath(toolSetId) handling from the controllers, which now pass decoded ids). ToolSetService.setResourceAuthStatuses is removed; all callers go through the enricher.

KirylKurnosenka and others added 2 commits September 14, 2026 21:24
…Enricher #1928

Renames ExternalServiceStatusEnricher to ResourceAuthStatusEnricher and moves
toolset status enrichment there (enrichToolSet), absorbing the id
encode/decode handling from the controllers. ToolSetService.setResourceAuthStatuses
is removed; ConfigResourceController, ResourceController and ToolSetController now
go through the enricher. ConfigResourceController also injects
ResourceAuthSettingsService via the constructor like its other services.
Unit tests follow the moved logic (ResourceAuthStatusEnricherTest).

Co-Authored-By: Claude Code <noreply@anthropic.com>
…nstructor #1928

The platform-route tests stubbed proxy.getAccessService() (and app/toolset
services) that the constructor no longer reads, tripping Mockito strict
stubs; only the config store is dereferenced at construction time.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@ai-dial-actions

Copy link
Copy Markdown
Contributor

Dependency Review

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

Scanned Files

None

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.

[Assets-Toolsets] Login status not updated after successful login, Log out button not shown

3 participants