BIPAuto/docs/plans/2026-03-27-auth-refactoring-implementation.md

Auth Module Refactoring Implementation Plan

For Claude: REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.

Goal: Refactor utils/auth.py into a pure, decoupled authentication module with zero environment variable dependencies.

Architecture: Remove all environment variable reading from auth.py, make all parameters required, remove the close_session() wrapper function. Test scripts will explicitly load configuration and pass all parameters.

Tech Stack: Python 3.x, Playwright, pytest

---

Task 1: Update utils/auth.py imports

Files:

• Modify: utils/auth.py:1-23

Step 1: Remove unused imports

Remove the following imports that are only used for environment variable access:

• import os (line 5)
• from dotenv import load_dotenv (line 10)
• from pathlib import Path (line 6)

The final imports should be:

"""
Authentication module - Responsible for Yonyou BIP system login and logout operations
"""

from playwright.sync_api import Playwright, Browser, BrowserContext, Page, Frame

Step 2: Remove module-level environment setup

Delete lines 10-22 (BASE_DIR, load_dotenv, browsers_path configuration).

Step 3: Verify syntax

Run: python -m py_compile utils/auth.py Expected: No syntax errors (function definitions will fail later steps, that's OK)

Step 4: Commit

git add utils/auth.py
git commit -m "refactor: remove environment variable imports from auth module

- Remove os, dotenv, and pathlib imports
- Remove module-level environment setup code
- Prepare for pure function implementation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 2: Refactor login() function signature

Files:

• Modify: utils/auth.py:25-52

Step 1: Update function signature to remove default parameters

Change the login() function signature from:

def login(
    playwright: Playwright,
    username: str = None,
    password: str = None,
    url: str = None,
    headless: bool = None,
    ignore_https_errors: bool = None,
    verbose: bool = True,
) -> tuple[Browser, BrowserContext, Page, Frame]:

To:

def login(
    playwright: Playwright,
    username: str,
    password: str,
    url: str,
    headless: bool,
    ignore_https_errors: bool,
    verbose: bool = True,
) -> tuple[Browser, BrowserContext, Page, Frame]:

Step 2: Update docstring

Replace the docstring (lines 34-52) with:

    """
    Login to Yonyou BIP system

    Args:
        playwright: Playwright instance
        username: Username (required)
        password: Password (required)
        url: Complete login page URL (required)
        headless: Whether to use headless mode (required)
        ignore_https_errors: Whether to ignore HTTPS errors (required)
        verbose: Whether to print detailed logs (default: True)

    Returns:
        tuple: (browser, context, page, main_frame)
            - browser: Browser instance
            - context: Browser context
            - page: Page object
            - main_frame: Main iframe after login (forwardFrame)
    """

Step 3: Verify syntax

Run: python -m py_compile utils/auth.py Expected: No syntax errors

Step 4: Commit

git add utils/auth.py
git commit -m "refactor: update login() signature to require all parameters

- Remove default values for username, password, url, headless, ignore_https_errors
- Update docstring to reflect required parameters
- Maintain backward compatibility with 4-tuple return value

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 3: Remove environment variable logic from login() body

Files:

• Modify: utils/auth.py:53-77

Step 1: Remove environment variable reading code

Delete lines 56-76 that contain:

• username = username or os.getenv("ERP_USERNAME")
• password = password or os.getenv("ERP_PASSWORD")
• URL construction logic with os.getenv("ERP_URL")
• headless default handling with os.getenv("ERP_HEADLESS")
• ignore_https_errors default handling with os.getenv("ERP_IGNORE_HTTPS_ERRORS")

Step 2: Add parameter validation

After the imports section (after the docstring), add validation:

    # Validate required parameters
    if not username:
        raise ValueError("username is required")
    if not password:
        raise ValueError("password is required")
    if not url:
        raise ValueError("url is required")

Step 3: Verify the login() function body

The function body should now start directly with:

    import time

    # Validate required parameters
    if not username:
        raise ValueError("username is required")
    if not password:
        raise ValueError("password is required")
    if not url:
        raise ValueError("url is required")

    # Launch browser
    browser = playwright.chromium.launch(headless=headless)
    # ... rest of function unchanged

Step 4: Verify syntax

Run: python -m py_compile utils/auth.py Expected: No syntax errors

Step 5: Commit

git add utils/auth.py
git commit -m "refactor: remove environment variable reading from login()

- Remove all os.getenv() calls from login() function
- Remove URL construction logic (caller provides complete URL)
- Add parameter validation with clear error messages
- Function is now pure with no side effects

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 4: Remove close_session() function

Files:

• Modify: utils/auth.py:161-177

Step 1: Delete the close_session() function

Remove lines 161-177, the entire close_session() function:

def close_session(browser: Browser, context: BrowserContext) -> None:
    """
    Close browser session

    Args:
        browser: Browser instance
        context: Browser context
    """
    # Check if auto-close browser is enabled
    auto_close = os.getenv("ERP_AUTO_CLOSE_BROWSER", "true").lower() in ("true", "1", "yes")

    if auto_close:
        context.close()
        browser.close()
    else:
        print("Note: Browser not auto-closed (ERP_AUTO_CLOSE_BROWSER=false)")

Step 2: Verify auth.py still has only two functions

Run: grep "^def " utils/auth.py Expected output:

def login(
def logout(

Step 3: Verify syntax

Run: python -m py_compile utils/auth.py Expected: No syntax errors

Step 4: Commit

git add utils/auth.py
git commit -m "refactor: remove close_session() function

- Remove close_session() wrapper function entirely
- Callers now directly manage browser/context lifecycle
- Reduces module to pure authentication functions only

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 5: Update tests/test_login.py to use new API

Files:

• Modify: tests/test_login.py:13,37-39,62-69

Step 1: Update import line

The import on line 13 stays the same:

from utils.auth import login, logout, close_session

Change to:

from utils.auth import login, logout

Step 2: Add URL construction before login() call

After line 31 (after displaying configuration), add URL construction:

# Construct complete login URL
url = f"{os.getenv('ERP_URL').rstrip('/')}/yonbip/resources/uap/rbac/login/main/index.html"

Step 3: Update login() call with all required parameters

Replace lines 37-40:

        browser, context, page, main_frame = login(
            playwright=p,
            verbose=True
        )

With:

        browser, context, page, main_frame = login(
            playwright=p,
            username=os.getenv('ERP_USERNAME'),
            password=os.getenv('ERP_PASSWORD'),
            url=url,
            headless=os.getenv('ERP_HEADLESS', 'false').lower() in ('true', '1', 'yes'),
            ignore_https_errors=os.getenv('ERP_IGNORE_HTTPS_ERRORS', 'true').lower() in ('true', '1', 'yes'),
            verbose=True
        )

Step 4: Replace close_session() with direct calls

Replace lines 62-69:

        # Check if auto-close browser
        auto_close = os.getenv("ERP_AUTO_CLOSE_BROWSER", "true").lower() in ("true", "1", "yes")
        if not auto_close:
            print("\n[INFO] Browser will stay open for 10 seconds for manual observation...")
            time.sleep(10)

        # Close browser
        print("\n[CLEANUP] Closing browser session...")
        close_session(browser, context)

With:

        # Check if auto-close browser
        auto_close = os.getenv("ERP_AUTO_CLOSE_BROWSER", "true").lower() in ("true", "1", "yes")
        if not auto_close:
            print("\n[INFO] Browser will stay open for 10 seconds for manual observation...")
            time.sleep(10)

        # Close browser
        print("\n[CLEANUP] Closing browser session...")
        context.close()
        browser.close()

Step 5: Verify syntax

Run: python -m py_compile tests/test_login.py Expected: No syntax errors

Step 6: Commit

git add tests/test_login.py
git commit -m "refactor: update test_login.py to use new auth API

- Import only login and logout (remove close_session)
- Add explicit URL construction before login call
- Pass all required parameters to login()
- Replace close_session() with direct context.close() and browser.close()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 6: Run tests to verify functionality

Files:

• Test: tests/test_login.py, utils/auth.py

Step 1: Activate virtual environment

Run: source .venv/Scripts/activate Expected: Command prompt shows (.venv)

Step 2: Run login test

Run: python tests/test_login.py Expected: Test completes successfully with login/logout operations

Step 3: Verify no regressions

Check that the test output shows:

• Configuration display
• Successful login
• Successful logout
• Browser closes cleanly

Step 4: Run auth config test

Run: python tests/test_auth_config.py Expected: Configuration check completes (no changes needed in this file)

Step 5: Commit if any fixes needed

If tests revealed issues:

git add tests/test_login.py utils/auth.py
git commit -m "fix: address test failures in refactored auth module

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

If all tests pass, no commit needed for this step.

---

Task 7: Update CLAUDE.md documentation

Files:

• Modify: CLAUDE.md:24-35,41-46

Step 1: Update Environment Configuration section

Replace lines 24-35 with:

## Environment Configuration

Configuration is loaded from `.env` file in the project root. Never commit `.env` to version control - use `.env.example` as a template.

**Environment Variables:**
- `PLAYWRIGHT_BROWSERS_PATH` - Path to Playwright browser installation
- `ERP_URL` - Base URL for the ERP system
- `ERP_USERNAME` - Login username
- `ERP_PASSWORD` - Login password
- `ERP_HEADLESS` - Whether to run browser in headless mode (true/false)
- `ERP_IGNORE_HTTPS_ERRORS` - Whether to ignore HTTPS certificate errors (true/false)

Note: Test scripts are responsible for loading environment variables and passing configuration to utility functions.

Step 2: Update Module Structure section

Replace lines 41-46 with:

**`utils/auth.py`** - Core authentication module (pure functions)
- `login()` - Handles Yonyou BIP login with automatic force-login popup detection. Requires all parameters (username, password, url, headless, ignore_https_errors).
- `logout()` - Performs logout with confirmation dialog handling.
- Returns tuple: `(browser, context, page, main_frame)` where `main_frame` is the forwardFrame iframe.
- Callers are responsible for browser lifecycle management (context.close(), browser.close()).

Step 3: Add URL Construction Pattern

After line 52 (after Page Interaction Pattern section), add:

### URL Construction Pattern

Callers must construct the complete login URL before passing to `login()`:

```python
url = f"{os.getenv('ERP_URL').rstrip('/')}/yonbip/resources/uap/rbac/login/main/index.html"

**Step 4: Update Code Conventions section**

Replace line 84 (Environment-First convention) with:
```markdown
2. **Environment-First**: Test scripts load environment variables and explicitly pass configuration to utility functions. No hardcoded values in code.

Step 5: Verify documentation

Run: grep -n "os.getenv" CLAUDE.md Expected: Only in code examples, not as recommendations for module internals

Step 6: Commit

git add CLAUDE.md
git commit -m "docs: update CLAUDE.md for refactored auth module

- Clarify that auth module is pure with no environment access
- Document URL construction pattern for callers
- Update module structure documentation
- Emphasize caller responsibility for configuration

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 8: Create .env.example template

Files:

• Create: .env.example

Step 1: Create environment variable template

Create .env.example with:

# Playwright Configuration
PLAYWRIGHT_BROWSERS_PATH=path/to/playwright/browsers

# ERP System Configuration
ERP_URL=https://your-erp-system.com
ERP_USERNAME=your_username
ERP_PASSWORD=your_password

# Browser Behavior
ERP_HEADLESS=false
ERP_IGNORE_HTTPS_ERRORS=true

Step 2: Verify .env is in .gitignore

Run: grep .env .gitignore Expected: .env is listed

Step 3: Commit

git add .env.example
git commit -m "docs: add .env.example template

- Provide template for required environment variables
- Document all configuration options
- Help users set up their local environment

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

---

Task 9: Final verification and cleanup

Files:

• Verify: utils/auth.py, tests/, CLAUDE.md

Step 1: Verify no environment imports in auth.py

Run: grep -E "(os\.getenv|load_dotenv)" utils/auth.py Expected: No matches

Step 2: Verify all parameters are required

Run: grep "def login" utils/auth.py Expected: Function signature has no = None defaults except verbose=True

Step 3: Run all tests

Run: python tests/test_login.py && python tests/test_auth_config.py Expected: All tests pass

Step 4: Check documentation consistency

Run: grep -n "close_session" CLAUDE.md Expected: No mentions (function was removed)

Step 5: Final commit if needed

If any cleanup was done:

git add -A
git commit -m "chore: final cleanup for auth module refactoring

- Verify no environment dependencies remain
- Confirm all tests pass
- Documentation is consistent

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>"

Step 6: Push to remote

Run: git push origin master Expected: All commits pushed successfully

---

Summary

This refactoring achieves:

1. Pure Functions: auth.py has zero side effects or environment dependencies
2. Explicit Dependencies: All parameters required, no hidden behavior
3. Separation of Concerns: Configuration managed by callers, auth module handles business logic only
4. Backward Compatible: 4-tuple return value maintained for existing code
5. Tested: All tests updated and passing

Migration Impact: Low - only test scripts need updates, API changes are additive (making optional params required)