feat: add query arg for oidc expiration

[hugocasa]

No description provided.

[claude]

Claude finished @hugocasa's task —— View job

---

PR Review: Add OIDC Expiration Query Parameter

Summary

This PR adds the ability to customize OIDC token expiration times via an optional expires_in query parameter. The implementation is clean, maintains backward compatibility, and is consistently applied across Python and TypeScript clients as well as the OpenAPI specification.

---

General Observations

✅ Strengths

1. Consistent Implementation: The feature is properly implemented across all three client libraries (Python, TypeScript, and the OpenAPI spec), ensuring consistency across the entire stack.

2. Backward Compatibility: The expires_in/expiresIn parameter is optional in all implementations, maintaining backward compatibility with existing code that doesn't specify an expiration time.

3. Clean Code: The implementations are straightforward and follow the existing patterns in each client library. The code is readable and maintainable.

4. Documentation Started: The TypeScript client includes a JSDoc comment documenting the new parameter, which is good practice.

🔍 Areas for Improvement

1. OpenAPI Documentation: The OpenAPI spec is missing a description for the new expires_in parameter. This should include:

   • What the parameter does
   • That it's optional
   • Any reasonable bounds or constraints
   • Default behavior when not provided

2. Python Client Documentation: While the TypeScript client has JSDoc documentation, the Python client's get_id_token method could benefit from an updated docstring documenting the new parameter.

3. Type Consistency: Minor note on naming conventions - Python uses expires_in (snake_case) while TypeScript uses expiresIn (camelCase). This is correct for each language's conventions but worth noting for cross-language documentation.

4. Input Validation: Consider whether the backend implementation (in the EE repo based on the ref update) includes validation for the expires_in parameter:

   • Should there be a minimum value (e.g., 60 seconds)?
   • Should there be a maximum value to prevent excessive token lifetimes?
   • How are negative values or zero handled?

🔒 Security Considerations

1. Token Lifetime Control: Allowing custom expiration times is generally a good security practice as it enables the principle of least privilege (shorter-lived tokens for less sensitive operations). However, ensure the backend enforces reasonable bounds.

2. No Obvious Vulnerabilities: The changes don't introduce any obvious security issues. The parameter is passed through standard query parameters, which is appropriate for this use case.

⚡ Performance Considerations

No performance concerns. The changes are minimal and only add an optional parameter to existing endpoints.

---

Testing Instructions

To test the OIDC token expiration query parameter functionality, navigate to a workspace where OIDC is configured and create a script that calls the get_id_token function. Test both with and without specifying the expires_in/expiresIn parameter. For example, in a Python script, call wmill.get_id_token("your-audience") to verify the default behavior still works, then call wmill.get_id_token("your-audience", expires_in=300) to test with a custom 5-minute expiration. Similarly, in TypeScript, test wmill.getIdToken("your-audience") and wmill.getIdToken("your-audience", 300). Verify that the generated tokens have the expected expiration times by decoding the JWT payload and checking the exp claim.

---

Overall Assessment: This is a solid PR that adds useful functionality. The suggested improvements in the inline comments are mostly documentation enhancements and minor refinements rather than critical issues. The code quality is good and the implementation is sound.

---

[claude]

Good addition of the JSDoc comment for the new parameter!

One minor consistency note: The Python client uses expires_in: int | None while TypeScript uses expiresIn?: number. While this follows each language's conventions (snake_case for Python, camelCase for TypeScript), it's worth noting this difference in case it affects any cross-language documentation or API consistency guidelines your team follows.

[cloudflare-workers-and-pages]

Deploying windmill with    Cloudflare Pages

Latest commit:	b28c2ad
Status:	✅  Deploy successful!
Preview URL:	https://bea2e138.windmill.pages.dev
Branch Preview URL:	https://hc-oidc-exp-query-arg.windmill.pages.dev

View logs

[hugocasa]

/eeref

[windmill-internal-app]

Starting ee ref update...

View workflow run

[windmill-internal-app]

Successfully updated ee-repo-ref.txt

[hugocasa]

/eeref

[windmill-internal-app]

Starting ee ref update...

View workflow run

[windmill-internal-app]

Successfully updated ee-repo-ref.txt