feat: add OIN catalog browse and app-install tools - #75
Conversation
Outbound SCIM provisioning only works on a provisioning-capable app, which must be an instance of an OIN catalog app — provisioning capability is fixed by the catalog definition at install time and cannot be added to a plain custom SAML/OIDC app. create_application could not install one because the typed SDK serialization strips the catalog 'name' key from the request body. Add list_catalog_apps / get_catalog_app to browse the OIN catalog (find an app's 'name' and provisioning features) and install_oin_app to install an instance via a direct POST that preserves 'name'. With an installed provisioning-capable app, the existing provisioning tools (set_app_provisioning_connection, update_app_feature, etc.) then work against it. Catalog reads gate on okta.apps.read, install on okta.apps.manage.
BinoyOza-okta
left a comment
BinoyOza-okta
left a comment
There was a problem hiding this comment.
Hi @mjdavidson
Thanks for this — the create_application workaround for catalog name stripping is a real gap and this is a reasonable fix. A few things before merging:
- Branch is 11 commits behind
main, missing #90. Your base commit predates88c4c1a("standardize MCP tool responses to valid JSON"), which is exactly the commit that made@json_responsemandatory on every tool inapplications.py. That's why none of the new tools use it. Please rebase onto currentmain— a straight merge will conflict at the tail ofdeactivate_applicationsince that same commit touched the line right before where your new code is appended. - Add
@json_responseto all three new tools to match the now-current convention (@mcp.tool()→@require_scopes→@validate_ids(if applicable) →@json_response→async def). Your tools already return JSON-native dicts viajson.loads, so this won't change success-path behavior, but it standardizes the failure envelope (right now failures return ad hoc{"error": ...}instead of the shared_failure_envelopeshape) and gets youOKTA_MCP_INCLUDE_RAW=1debug support for free. list_catalog_appshas no pagination. The response object is discarded (_, response_body, err = await executor.execute(request)), so there's no way to read a Link-header cursor even if the catalog endpoint paginates (it's a large catalog — worth confirming against a live org). Every otherlist_*tool in this codebase routes throughbuild_query_params/paginate_all_results/create_paginated_response; please check whether this endpoint needs the same treatment.
Smaller note inline below on the duplicated request boilerplate. Nice test coverage on the "preserve name in body" behavior, which is the crux of the fix — a couple of gaps noted inline too.
|
|
||
| @mcp.tool() | ||
| @require_scopes("okta.apps.read") | ||
| async def list_catalog_apps(ctx: Context, q: Optional[str] = None, limit: Optional[int] = None) -> Any: |
There was a problem hiding this comment.
Missing @json_response above this (and on get_catalog_app / install_oin_app below) — see the general comment. This is the mandatory innermost decorator on every tool; your branch predates the commit that introduced it.
| logger.error(f"Error building catalog request: {err}") | ||
| return {"error": str(err)} | ||
|
|
||
| _, response_body, err = await executor.execute(request) |
There was a problem hiding this comment.
The response object is discarded here, so any Link-header pagination cursor from the catalog endpoint is unreachable. If /api/v1/catalog/apps paginates (worth checking against a live org — it's a large catalog), this will silently return only the first page with no has_more signal, unlike every other list_* tool in this file/codebase.
| assert result == {"error": "403 forbidden"} | ||
|
|
||
|
|
||
| class TestGetCatalogApp: |
There was a problem hiding this comment.
Could use one more test here: app_name with path-traversal characters (e.g. "../admin") should be rejected by @validate_ids before the API call — this is exercised for equivalent tools elsewhere in the test suite (see tests/test_validation.py and test_user_resources.py's test_invalid_user_id_rejected_before_api_call) and would be good to mirror here.
| return ctx | ||
|
|
||
|
|
||
| def _client_returning(body, execute_error=None): |
There was a problem hiding this comment.
All three error-path tests only cover the (None, None, err) tuple case; none exercise a raised exception (e.g. mock_get_client.side_effect = Exception(...)). Once @json_response is added, that path routes through _failure_envelope — worth a test confirming that shape.
| query_string = urlencode(params) | ||
| url = "/api/v1/catalog/apps" + (f"?{query_string}" if query_string else "") | ||
|
|
||
| executor = client.get_request_executor() |
There was a problem hiding this comment.
This create_request/execute/error-check sequence (lines ~489-501) is repeated near-verbatim in get_catalog_app and install_oin_app below — this raw-request pattern is new (it doesn't exist elsewhere in the codebase, which otherwise goes through the typed SDK client methods), so there's no established convention to match here. Worth factoring the three copies into one small helper, e.g. _catalog_request(client, method, url, body=None), since it's introduced fresh in this PR rather than being pre-existing duplication.
2 participants
Outbound SCIM provisioning only works on an instance of an OIN catalog app — provisioning capability is fixed by the catalog definition at install time and cannot be added to a plain custom app.
create_applicationcannot install one because the typed SDK serialization strips the catalognamekey. Addslist_catalog_apps/get_catalog_appto browse the OIN catalog andinstall_oin_appto install an instance via a direct POST that preservesname. Catalog reads gate onokta.apps.read, install onokta.apps.manage.Tests:
tests/test_oin_catalog.py.