Skip to content

feat(platform): add organization parameter to OAuth authorization redirect#550

Open
santi698 wants to merge 2 commits intomainfrom
feat/oauth-organization-parameter
Open

feat(platform): add organization parameter to OAuth authorization redirect#550
santi698 wants to merge 2 commits intomainfrom
feat/oauth-organization-parameter

Conversation

@santi698
Copy link
Copy Markdown
Collaborator

Summary

Add support for Auth0 organization-specific login flows via a new AIGNOSTICS_ORGANIZATION environment variable.

When set, the organization parameter is included in the OAuth 2.0 PKCE authorization URL, enabling users to authenticate into a specific Auth0 organization (useful in development and staging environments).

Changes

  • _settings.py: Added organization: str | None field, configurable via AIGNOSTICS_ORGANIZATION env var
  • _authentication.py: _perform_authorization_code_with_pkce_flow() now conditionally adds organization to the authorization URL parameters
  • authentication_test.py: Added unit test verifying the organization parameter is correctly passed through the full PKCE flow

Usage

export AIGNOSTICS_ORGANIZATION=my-org
uv run aignostics user login

Or via .env:

AIGNOSTICS_ORGANIZATION=my-org

…irect

Add optional `AIGNOSTICS_ORGANIZATION` environment variable that, when set,
includes the `organization` parameter in the OAuth 2.0 PKCE authorization URL.
This enables Auth0 organization-specific login flows in development and staging.

- Add `organization: str | None` field to `Settings`
- Conditionally include `organization` in `session.authorization_url()` params
- Add unit test verifying the parameter is passed through the PKCE flow
Copilot AI review requested due to automatic review settings April 20, 2026 10:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds support for Auth0 organization-scoped interactive OAuth login by introducing an optional organization setting (via AIGNOSTICS_ORGANIZATION) and conditionally appending it to the PKCE authorization URL.

Changes:

  • Add organization: str | None to platform settings (env-backed via the existing AIGNOSTICS_ prefix).
  • Update PKCE authorization URL construction to include organization when configured.
  • Add a unit test asserting the organization kwarg is passed to OAuth2Session.authorization_url.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/aignostics/platform/_settings.py Introduces optional organization setting to control Auth0 org login behavior.
src/aignostics/platform/_authentication.py Conditionally includes organization in the authorization URL parameters for PKCE.
tests/aignostics/platform/authentication_test.py Adds coverage ensuring organization propagates into the authorization URL call.

Comment thread tests/aignostics/platform/authentication_test.py
Comment thread src/aignostics/platform/_settings.py Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Files with missing lines Coverage Δ
src/aignostics/platform/_authentication.py 78.31% <100.00%> (+0.39%) ⬆️
src/aignostics/platform/_settings.py 85.79% <100.00%> (+0.08%) ⬆️

... and 7 files with indirect coverage changes

@olivermeyer
Copy link
Copy Markdown
Collaborator

Thanks for the PR, some thoughts:

  • This needs a CR as per this announcement - did you already create one, and if not could you?
  • The SPEC-APPLICATION-SERVICE.md needs updating (at least here, maybe elsewhere too) - could you review and update wherever needed?
  • SonarQube quality gate should pass
  • Tests on window should pass if you rebase to catch these changes, however not strictly needed to merge

@santi698
Copy link
Copy Markdown
Collaborator Author

* The SPEC-APPLICATION-SERVICE.md needs updating (at least [here](https://github.com/aignostics/python-sdk/blob/main/specifications/SPEC-APPLICATION-SERVICE.md#62-environment-variables), maybe elsewhere too) - could you review and update wherever needed?

I think you mean SPEC_PLATFORM_SERVICE.md

@olivermeyer
Copy link
Copy Markdown
Collaborator

* The SPEC-APPLICATION-SERVICE.md needs updating (at least [here](https://github.com/aignostics/python-sdk/blob/main/specifications/SPEC-APPLICATION-SERVICE.md#62-environment-variables), maybe elsewhere too) - could you review and update wherever needed?

I think you mean SPEC_PLATFORM_SERVICE.md

You're right, seems like SPEC-APPLICATION-SERVICE only covers what happens once the token is acquired. So platform is the one 👍

@santi698
Copy link
Copy Markdown
Collaborator Author

This needs a CR as per this announcement - did you already create one, and if not could you?This needs a CR as per this announcement - did you already create one, and if not could you?

CR HERE: https://aignx.atlassian.net/browse/PYSDK-77

@santi698 santi698 force-pushed the feat/oauth-organization-parameter branch from 289fe59 to c214653 Compare April 20, 2026 15:26
Copilot AI review requested due to automatic review settings April 20, 2026 15:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment on lines 414 to 418
| `api_root` | str | `https://platform.aignostics.com` | Base URL of Aignostics API | Yes |
| `audience` | str | Environment-specific | OAuth audience claim | Yes |
| `scope` | str | `offline_access` | OAuth scopes required | Yes |
| `organization` | str | None | Auth0 organization for OAuth| No |
| `cache_dir` | str | User cache directory | Directory for token storage | No |
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

In the configuration parameters table, organization is documented as type str but the default is None and the code defines it as optional (str | None). Please update the table to reflect the optional type (and consider formatting None/spacing consistently) so the spec doesn’t mislead users.

Copilot uses AI. Check for mistakes.
…fication

Add AIGNOSTICS_ORGANIZATION to the platform service specification:
- Add organization parameter to Configuration Parameters (section 6.1)
- Add AIGNOSTICS_ORGANIZATION environment variable (section 6.2)
- Update PKCE Flow description to document organization parameter usage
@santi698 santi698 force-pushed the feat/oauth-organization-parameter branch from c214653 to ee93e37 Compare April 20, 2026 15:40
Comment on lines +209 to +211
organization_id: Annotated[
str | None, Field(description="Optional Auth0 organization ID parameter for the /authorize OAuth endpoint")
] = None
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The organization_id setting expects the AIGNOSTICS_ORGANIZATION_ID environment variable, but documentation specifies AIGNOSTICS_ORGANIZATION, causing the feature to silently fail.
Severity: CRITICAL

Suggested Fix

Align the code and documentation. Either rename the organization_id field to organization, or add a validation_alias to the Field definition to accept AIGNOSTICS_ORGANIZATION. For example: Field(..., validation_alias='AIGNOSTICS_ORGANIZATION'). Ensure all documentation is updated to reflect the final variable name.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/aignostics/platform/_settings.py#L209-L211

Potential issue: The Pydantic `Settings` class defines a field `organization_id`, which
by default maps to the environment variable `AIGNOSTICS_ORGANIZATION_ID` due to the
`AIGNOSTICS_` prefix. However, the documentation instructs users to set
`AIGNOSTICS_ORGANIZATION`. When users follow the documentation, the `organization_id`
setting will default to `None` because the expected environment variable is not found.
This causes the organization-specific login feature to silently fail, as the
`organization` parameter is never added to the OAuth authorization request.

Also affects:

  • specifications/SPEC_PLATFORM_SERVICE.md:433~439

Did we get this right? 👍 / 👎 to inform future reviews.

@sonarqubecloud
Copy link
Copy Markdown

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