Skip to content

Conversation

@denizs
Copy link
Contributor

@denizs denizs commented Nov 5, 2025

Description

Documentation

Once a new version is released, one could update the code examples that are currently limited to authkit-nextjs and express.

[x] Yes

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

@denizs
Copy link
Contributor Author

denizs commented Nov 6, 2025

@mattgd - I'd like to get this contribution in, but I'm unsure of the next steps. The PR template tells me link a PR to corresponding docs, however, searching for docs in your org's repos didn't yield any result. Could please point me into the right direction?

Copy link
Member

@nholden nholden left a comment

Choose a reason for hiding this comment

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

👋 Hey, @denizs, thanks for this contribution!

I pulled this PR down and tried getting it working with an example app locally, and I bumped into some issues. First, I saw a ModuleNotFoundError: No module named 'workos.types.api_keys', which Claude Code suggests could reflect a naming conflict between a type and a class. Second, I saw that the API endpoint wasn't being called correctly, as I noted inline.

I'd like to get this contribution in, but I'm unsure of the next steps.

Could you get this change working in an app on your machine? After you've done that, please ping me for another review.

@denizs
Copy link
Contributor Author

denizs commented Nov 13, 2025

Hey @nholden sorry for that very unpleasant review. I should've mentioned the PR is by far not ready for review and that I'm primarily wondering what to do about the following sentence in the PR template:

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

It sounded like I had to first create another PR somewhere else before getting this one reviewed.

I've cleaned up the AI slop and can confirm it's working on my machine ™️. I've used the API key widget to issue an API key for our application and then successfully validated it:

In [1]: from workos import WorkOSClient

In [2]: workos_client = WorkOSClient(
   ...:     client_id="client_[..]",
   ...:     api_key="sk_test_[..]"
   ...: )

In [3]: workos_client.api_keys.validate_api_key(value="sk_[..]")
Out[3]: ApiKey(object='api_key', id='api_key_[..]', owner=ApiKeyOwner(type='organization', id='org_[..]'), name='test', obfuscated_value='...', last_used_at='2025-11-13T08:38:26.462Z', permissions=[], created_at='2025-11-13T08:00:17.478Z', updated_at='2025-11-13T08:15:11.604Z')

@denizs denizs marked this pull request as ready for review November 13, 2025 08:42
@denizs denizs requested a review from a team as a code owner November 13, 2025 08:43
@denizs denizs requested review from awolfden and nholden November 13, 2025 08:43
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 13, 2025

Greptile Overview

Greptile Summary

This PR adds API key validation functionality to the workos-python SDK, allowing developers to programmatically validate API keys.

Key Changes:

  • Added new ApiKeys and AsyncApiKeys modules with validate_api_key() method
  • Created ApiKey and ApiKeyOwner Pydantic models for type safety
  • Integrated modules into both sync (SyncClient) and async (AsyncClient) clients
  • Implemented comprehensive test coverage using the sync_and_async test pattern
  • Followed established SDK patterns for module structure, HTTP client usage, and error handling

Implementation Quality:

  • Properly handles authentication errors by raising AuthenticationException on 401 responses
  • Uses Protocol classes for type hints and maintains dual sync/async support
  • Follows the SDK's established patterns (lazy initialization, module structure, test patterns)
  • API key path extracted as constant API_KEY_VALIDATION_PATH for maintainability

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • The implementation follows all established SDK patterns perfectly, includes comprehensive tests, properly handles errors, and introduces no security concerns. All custom rules were checked (no sensitive data logging, no SQL injection, proper error handling).
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
workos/api_keys.py 5/5 Added new API keys module with validate_api_key method following established patterns with sync/async support
workos/types/api_keys/api_keys.py 5/5 Defined ApiKey and ApiKeyOwner Pydantic models matching API response structure
tests/test_api_keys.py 5/5 Comprehensive tests for both sync/async variants with valid and invalid key scenarios

Sequence Diagram

sequenceDiagram
    participant Client as SDK Client
    participant ApiKeys as ApiKeys Module
    participant HTTP as HTTP Client
    participant API as WorkOS API

    Client->>ApiKeys: validate_api_key(value="sk_...")
    ApiKeys->>HTTP: request(path="api_keys/validations", method=POST, json={value: "sk_..."})
    HTTP->>API: POST /api_keys/validations
    
    alt Valid API Key
        API-->>HTTP: 200 OK {api_key: {...}}
        HTTP-->>ApiKeys: response_json
        ApiKeys->>ApiKeys: ApiKey.model_validate(response["api_key"])
        ApiKeys-->>Client: ApiKey object
    else Invalid API Key
        API-->>HTTP: 401 Unauthorized
        HTTP->>HTTP: raise AuthenticationException
        HTTP-->>ApiKeys: AuthenticationException
        ApiKeys-->>Client: AuthenticationException
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

9 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Member

@nholden nholden left a comment

Choose a reason for hiding this comment

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

Thanks, @denizs! Left a couple of questions for you.

I'm primarily wondering what to do about the following sentence in the PR template:

If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.

It sounded like I had to first create another PR somewhere else before getting this one reviewed.

Sorry, WorkOS needs to fix this! You did everything right. Our docs aren't open source, so I can make the docs change for you after we release this change.

@denizs denizs requested a review from nholden December 4, 2025 13:44
Copy link
Member

@nholden nholden left a comment

Choose a reason for hiding this comment

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

Thank you, @denizs! This worked great for me. I'm going to see if I can loop in a Python expert for another review, but we should get this in a release relatively quickly.

@denizs
Copy link
Contributor Author

denizs commented Dec 4, 2025

Thanks @nholden! And sorry for the slight delay - I was on vacation

Copy link
Contributor

@mattgd mattgd left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for adding this and apologies for the delay.

@nholden nholden removed the request for review from awolfden December 5, 2025 18:00
@nholden nholden merged commit 7747508 into workos:main Dec 5, 2025
8 checks passed
@nholden nholden mentioned this pull request Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants