feat(auth): OIDC device flow & API audience for library (sc-15741)#501
feat(auth): OIDC device flow & API audience for library (sc-15741)#501
Conversation
|
|
13aa151 to
9bc78f0
Compare
|
Pull requests must include at least one of the required labels: |
1 similar comment
|
Pull requests must include at least one of the required labels: |
a628c1c to
91fe3fc
Compare
|
Pull requests must include at least one of the required labels: |
|
Pull requests must include at least one of the required labels: |
2 similar comments
|
Pull requests must include at least one of the required labels: |
|
Pull requests must include at least one of the required labels: |
- vm.init: issuer, client_id, scope, audience; env VM_OIDC_AUDIENCE - Device authorize/token requests send audience for RS256 API tokens (e.g. Auth0) - credentials_store + oidc_device; normalize issuer/client_id/audience; cache keys - ValidMindAuthError; CodeQL-safe device prompt - Unit tests; poetry.lock update - fix(lint): reduce poll_device_token complexity (flake8 C901) - fix(pandas): replace deprecated is_categorical_dtype checks (pandas 2.x) - test: skip xgboost-dependent tests when extra not installed - fix(async): use asyncio.run when no running loop in run_async (Python 3.9+ CI) Co-authored-by: Cursor <cursoragent@cursor.com>
91fe3fc to
421e1ed
Compare
| api_key: Optional[str] = None, | ||
| api_secret: Optional[str] = None, | ||
| api_host: Optional[str] = None, | ||
| api_url: Optional[str] = None, |
There was a problem hiding this comment.
Why not derive api_url from api_host?
No need to offer a new argument to a user if they both accomplish the same.
There was a problem hiding this comment.
sure, the ticket actually had api_url specified in the story hence why it was added
There was a problem hiding this comment.
right now they are both the same (oidc device flow will still work passing api_host and not api_url), api_url seems better naming though, since we were using api_host as actually being the tracking url, with the full tracking namespace route. I suggest we leave both, and we deprecate api_host on followup PRs
cachafla
left a comment
There was a problem hiding this comment.
Looking good 👍 I'd suggest an accompanying notebook to allow users to test this without changing an existing notebook.
Maybe we can copy quickstart_model_documentation.ipynb and create a new one called quickstart_model_documentation_oidc_device_flow.ipynb?
Only difference in content is an explanation at the beginning about what parameters to pass to vm.init() and how to complete the flow.
|
I'll dig into this more tomorrow but when trying to use an entra token the backend gives me the error below. I think the token is generated correctly since it gives me a code to put into a web browser. |
- Add quickstart_model_documentation_oidc_device_flow.ipynb with vm.init(issuer, client_id, ...) and device-flow UX. - Add 1-set_up_validmind_oidc_device_flow.ipynb variant with links to docs. - Add docs/oidc-device-flow-release-notes.md (OIDC library release notes). Co-authored-by: Cursor <cursoragent@cursor.com>
done |
There was a problem hiding this comment.
No need to provide a demo notebook for this one. The quickstart is good enough.
There was a problem hiding this comment.
No need to refer to a separate file here:
...see [OIDC device flow release notes](../../docs/oidc-device-flow-release-notes.md).
Better to prevent this kind of linking since that will force us to carry 2 files around: this one and the markdown. Better to link it to a ValidMind docs page if needed.
There was a problem hiding this comment.
This should also be removed in the #### Configure vm.init() for OIDC section.
There was a problem hiding this comment.
issuer="https://login.microsoftonline.com/<tenant-id>/v2.0", # your IdP issuer (OpenID discovery)
Why not use ValidMind's prod URL so it can be tested without looking up the prod login authority?
There was a problem hiding this comment.
@cachafla I didn’t see valimind url in any notebooks vm.init so I assumed we don’t want to include this here. I will add issuer per your request (but leave client id empty, maybe we want to add this somewhere in the app instead)
|
To get I also verified that I can call |
Unit tests |
|
@mdeyell-valid-mind with these changes for Entra, did you have a chance to ensure it keeps working with Auth0? |
I haven't ran this for auth0 yet but i'll check |
mdeyell-valid-mind
left a comment
There was a problem hiding this comment.
works on my machine
|
I tested with entra, auth0, and api keys |
cachafla
left a comment
There was a problem hiding this comment.
Approving now but the quickstart notebook still needs to be updated to not refer to a separate file (docs/oidc-device-flow-release-notes.md) because we need to ensure this notebook is portable and does not have any extra dependencies.
…ickstart Keep only the OIDC quickstart; avoid linking to repo markdown. Point readers to ValidMind Library docs and admins for audience/scope. Co-authored-by: Cursor <cursoragent@cursor.com>
PR SummaryThis PR adds comprehensive support for OIDC device flow authentication as an alternative to API key based authentication. The following changes and enhancements have been introduced:
Overall, the PR functionally enhances authentication flexibility, provides thorough documentation and guidance for users, and includes an extensive suite of tests to ensure reliability and correctness. Test Suggestions
|
Pull Request Description
Backend PR - https://github.com/validmind/backend/pull/3075
What and why?
This PR adds OAuth 2.0 / OIDC device authorization flow support to the ValidMind Python library so notebooks can authenticate with a Bearer token instead of only API keys.
Behavior:
vm.init(..., issuer=..., client_id=..., scope=..., audience=...)runs RFC 8628 device login, caches tokens under~/.validmind/credentials.json, and sendsAuthorization: Bearer ...to the tracking API.audience(or envVM_OIDC_AUDIENCE) is passed to the device authorize and token endpoints so Auth0 (and similar IdPs) can issue RS256 API access tokens for the resource identifier that matches the backendapi_audience.normalize_issuer,normalize_client_id, andnormalize_audiencestrip common wrapping quotes from copied.env/ notebook values.validmind/oidc_device.py,validmind/credentials_store.py. Auth failures raiseValidMindAuthError(extends existing error hierarchy).Before: Library authentication required
api_key/api_secretonly.After: Either API keys or OIDC device flow parameters can be used (mutually exclusive).
How to test
pip install -e .from this repo; restart the Jupyter kernel.api_audience; authorize the app for that API.vm.init( api_host='', model='<your model id>', document="documentation", issuer='', client_id='', audience='', )(empty strings prevent env API keys from mixing with OIDC during local tests).What needs special review?
VM_OIDC_AUDIENCE) vs platform docs.Dependencies, breaking changes, and deployment notes
audienceonvm.initfor RS256 API-token flows.Release notes
Enhancement: Optional OAuth device login for
vm.initviaissuer,client_id, optionalscope, and optionalaudience(Auth0 API Identifier). Environment variableVM_OIDC_AUDIENCEsupported. Tokens cached under~/.validmind/credentials.json.Suggested label:
enhancement,environment-variablesChecklist