-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: add multiple provider support #3089
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Disclaimer: This review was made by a crew of AI Agents. Code Review Comment for PR #3089OverviewThis PR introduces significant changes to the authentication mechanisms within the CrewAI CLI by removing the separate signup command and updating the overall authentication flow. The transition to multiple authentication providers enhances flexibility, but it is crucial to ensure that the user experience remains straightforward. Code Quality FindingsSecurity Improvements
Current Implementation: def _validate_and_save_token(self, token_data: Dict[str, Any]) -> None:
jwt_token_data = {
"jwt_token": token_data["id_token"],
...
} Suggested Improvement: def _validate_and_save_token(self, token_data: Dict[str, Any]) -> None:
if not token_data.get("id_token"):
raise ValueError("Invalid token data: missing id_token")
jwt_token_data = {
"jwt_token": token_data["id_token"],
...
}
# Consider adding expiration validation Integrating checks for required fields improves resilience against malformed input. Error Handling
Improved Implementation: def _poll_for_token(self, device_code_data: Dict[str, Any], client_id: str, token_poll_url: str) -> None:
...
while attempts < max_attempts:
try:
...
if response.status_code == 429: # Rate limiting
time.sleep(2 ** attempts) # Exponential backoff
...
except requests.exceptions.RequestException as e:
console.print(f"Network error: {str(e)}") This approach ensures the system handles transient errors gracefully. Configuration Management
Suggested Implementation: def clear(self) -> None:
try:
if self.config_path.exists():
self.config_path.unlink()
self.__init__(self.config_path) # Reinitialize
except Exception as e:
raise ConfigurationError(f"Failed to clear settings: {str(e)}") This not only enhances functionality but preserves user data integrity. Documentation ImprovementsDocumentation must reflect changes accurately:
Lessons from Related PRs
Recommendations for Improvement
ConclusionThe proposed changes significantly enhance CrewAI's authentication system, contributing to improved user experience and security. A focused user communication strategy and meticulous testing will be essential to facilitate these transitions. Overall, the direction of this PR is promising, and with the implementation of the suggested improvements, it will further solidify the robustness of the authentication mechanisms. |
Multiple provider support (Auth0 to WorkOS migration)
This PR parametrizes the methods used for authenticating with CrewAI Enterprise platform and also the method to validate the JWT token received from the request.
crewai signup
command because it did the same thing as thecrewai login
command. Now justcrewai login
is used.clear
method to theSettings
model that erases all stored user settingsclear
every time an user performscrewai login
(solves a bug that could happen when trying to reauthenticate but the user organization was deleted or changed)Important
This should be merged alongside