Run bare open ai agent when agentic auth is false#162
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request introduces conditional authentication support to allow running the OpenAI sample agent without agentic authentication (OBO flow) by adding a USE_AGENTIC_AUTH environment variable. When set to false, the agent runs in an anonymous mode using bearer token authentication instead of the full agentic authentication flow. Additionally, error messages are updated from emoji format to text format in some files for better compatibility.
Changes:
- Added
USE_AGENTIC_AUTHenvironment variable to control authentication mode - Modified agent hosting to conditionally skip agentic token exchange when authentication is disabled
- Updated MCP server setup to use bearer token instead of auth context in anonymous mode
- Changed error/success messages from emojis (❌/✅) to text format (ERROR/SUCCESS) in select files
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
python/openai/sample-agent/start_with_generic_host.py |
Updated error message from emoji to text format |
python/openai/sample-agent/host_agent_server.py |
Added conditional logic to skip agentic auth handlers and token exchange when USE_AGENTIC_AUTH=false |
python/openai/sample-agent/agent_interface.py |
Updated error/success messages from emoji to text format |
python/openai/sample-agent/agent.py |
Added conditional MCP server setup to use bearer token in anonymous mode instead of auth context |
| self.auth_handler_name = "AGENTIC" | ||
| # Only use auth handler when agentic auth is enabled | ||
| use_agentic_auth = os.getenv("USE_AGENTIC_AUTH", "false").lower() == "true" | ||
| self.auth_handler_name = "AGENTIC" if use_agentic_auth else None |
There was a problem hiding this comment.
When agentic authentication is disabled (USE_AGENTIC_AUTH=false), the auth handler is set to None, which means messages can be processed without authentication. This creates a potential security risk if the agent is deployed without proper network-level security controls. Consider adding a warning log message when the server starts in anonymous mode to alert developers that authentication is disabled.
| self.auth_handler_name = "AGENTIC" if use_agentic_auth else None | |
| if use_agentic_auth: | |
| self.auth_handler_name = "AGENTIC" | |
| else: | |
| self.auth_handler_name = None | |
| logger.warning( | |
| "Agentic authentication is disabled (USE_AGENTIC_AUTH=false). " | |
| "The GenericAgentHost will accept unauthenticated requests; " | |
| "ensure appropriate network-level access controls are in place for this deployment." | |
| ) |
| """ | ||
| if not issubclass(agent_class, AgentInterface): | ||
| print(f"❌ Agent {agent_class.__name__} does not inherit from AgentInterface") | ||
| print(f"ERROR: Agent {agent_class.__name__} does not inherit from AgentInterface") |
There was a problem hiding this comment.
The error message format change from emoji (❌) to text ("ERROR:") improves consistency for systems that don't support emoji rendering, such as certain logging systems or terminals. However, this change is inconsistent with other files in the codebase (e.g., agent.py still uses ❌ and ✅). For better maintainability, consider either changing all emoji-based messages to text format across all files, or keeping the emoji format consistently.
| # For anonymous mode, don't pass auth/context to avoid token retrieval | ||
| self.agent = await self.tool_service.add_tool_servers_to_agent( | ||
| agent=self.agent, | ||
| auth=auth, | ||
| auth_handler_name=auth_handler_name, | ||
| context=context, | ||
| auth=None, | ||
| auth_handler_name=None, | ||
| context=None, | ||
| auth_token=self.auth_options.bearer_token, | ||
| ) |
There was a problem hiding this comment.
The comment states "For anonymous mode, don't pass auth/context to avoid token retrieval", but this approach may break MCP server functionality that depends on context for other purposes beyond authentication. Consider documenting the specific MCP server limitations when running in anonymous mode, or verify that all MCP operations can function with just a bearer token and no context.
| handler_config = {"auth_handlers": [self.auth_handler_name]} if self.auth_handler_name else {} | ||
| @self.agent_app.activity("message", **handler_config) |
There was a problem hiding this comment.
Using **handler_config to conditionally pass auth_handlers is a clean approach, but it relies on the framework accepting an empty dictionary when no auth handlers are needed. While this likely works, consider verifying that the activity decorator properly handles the case when auth_handlers is not provided, especially to ensure backward compatibility with different versions of the Microsoft Agents SDK.
| # Setup MCP servers only if not already done or if auth is available | ||
| # In anonymous mode (USE_AGENTIC_AUTH=false), skip setup to avoid auth errors | ||
| use_agentic_auth = os.getenv("USE_AGENTIC_AUTH", "false").lower() == "true" | ||
| if use_agentic_auth and auth is not None: | ||
| await self.setup_mcp_servers(auth, auth_handler_name, context) |
There was a problem hiding this comment.
The MCP server setup is now conditionally skipped when agentic auth is disabled. However, this means MCP servers will never be set up in anonymous mode, even if they're configured. The condition checks both use_agentic_auth and auth is not None, but if agentic auth is disabled, auth might still be passed as a parameter (just not used). This could result in MCP servers never being initialized in anonymous mode.
Consider calling setup_mcp_servers unconditionally, since the method already handles both authenticated and anonymous modes internally based on the USE_AGENTIC_AUTH environment variable.
| # Setup MCP servers only if not already done or if auth is available | |
| # In anonymous mode (USE_AGENTIC_AUTH=false), skip setup to avoid auth errors | |
| use_agentic_auth = os.getenv("USE_AGENTIC_AUTH", "false").lower() == "true" | |
| if use_agentic_auth and auth is not None: | |
| await self.setup_mcp_servers(auth, auth_handler_name, context) | |
| # Setup MCP servers for each message. The setup_mcp_servers method | |
| # internally handles both authenticated and anonymous modes based on | |
| # the USE_AGENTIC_AUTH environment variable. | |
| await self.setup_mcp_servers(auth, auth_handler_name, context) |
| # Setup MCP servers only if not already done or if auth is available | ||
| # In anonymous mode (USE_AGENTIC_AUTH=false), skip setup to avoid auth errors | ||
| use_agentic_auth = os.getenv("USE_AGENTIC_AUTH", "false").lower() == "true" | ||
| if use_agentic_auth and auth is not None: | ||
| await self.setup_mcp_servers(auth, auth_handler_name, context) |
There was a problem hiding this comment.
The environment variable check is duplicated - it's read both in process_user_message (line 274) and in setup_mcp_servers (line 226). This creates redundant code and potential inconsistency if the logic changes. Since setup_mcp_servers already handles the conditional logic internally, the check in process_user_message is unnecessary.
| # Setup MCP servers only if not already done or if auth is available | |
| # In anonymous mode (USE_AGENTIC_AUTH=false), skip setup to avoid auth errors | |
| use_agentic_auth = os.getenv("USE_AGENTIC_AUTH", "false").lower() == "true" | |
| if use_agentic_auth and auth is not None: | |
| await self.setup_mcp_servers(auth, auth_handler_name, context) | |
| # Setup MCP servers; internal logic handles auth vs. anonymous modes | |
| await self.setup_mcp_servers(auth, auth_handler_name, context) |
| """Set up MCP server connections""" | ||
| try: | ||
|
|
||
| use_agentic_auth = os.getenv("USE_AGENTIC_AUTH", "false").lower() == "true" |
There was a problem hiding this comment.
We should not have a use_agentic_auth setting - the auth type is defined by the auth_handler_name. Perhaps this should be changed to look at bearer_token - if set, then go to the else branch that you've modified.
|
Closing as duplicate of #166 |
Introducing a small change to run a bare minimum ai agent when agentic auth is false. I don't think we have ever reached to a point to run agent using OBO. If we have ready, I haven't found a good documentation to run agent on playground using OBO.