feat: app registration authentication for equnor pi webapi

[asmfstatoil]

No description provided.

[asmfstatoil]

Arbeider med PI-teamet for å få autentisering til å virke med app-registrering. Kan hende det vert endring i client app-id

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (6)

tagreader/web_handlers.py:86

• http.cookiejar.Cookie cannot be constructed with only name/value/domain/... keyword args; the constructor requires many required fields (e.g., version, port, domain_specified, etc.). This will raise TypeError when converting Playwright cookies. Consider building a requests.cookies.RequestsCookieJar and using .set(...) instead.

def browser_context_to_cookiejar(
    context: BrowserContext, domain_filter: Optional[str] = None
):
    """Convert Playwright browser context cookies to a Requests cookie jar."""
    cookie_jar = CookieJar()

tagreader/web_handlers.py:167

• Launching Playwright with channel="msedge" unconditionally is Windows/Edge-specific and will fail on macOS/Linux. Gate the channel selection by platform (or omit channel outside Windows) to keep the fallback auth path from crashing on non-Windows hosts.

    with sync_playwright() as playwright:
        browser = playwright.chromium.launch(headless=False, channel="msedge")

tagreader/web_handlers.py:71

• browser_cookie3.edge() returns a CookieJar that doesn't support len(), so len(cookies) == 0 will raise TypeError at runtime. Convert the jar to a list (or use any(...)) before checking emptiness.

This issue also appears on line 82 of the same file.

def f5_check_browser_cookie():
    cookies = browser_cookie3.edge(domain_name=".equinor.com")
    if len(cookies) == 0:
        raise ConnectionError(
            "No cookies found for .piwebapi.equinor.com. Please log in to the F5 VPN using Microsoft Edge and try again."

tagreader/web_handlers.py:163

• STATE_FILE is written to the current working directory, which can be read-only (services/containers) and is easy to accidentally leak even with .gitignore. Store it under a user-scoped directory and ensure it exists.

This issue also appears on line 166 of the same file.

def ensure_f5_authenticated_context():
    STATE_FILE = Path(f"f5_{get_user_name()}_piwebapi_session.json")

tagreader/web_handlers.py:199

• get_auth_pi() can return cookie-based auth objects (Cookie/CookieJar/list of cookie dicts), but the return annotation only includes HTTPKerberosAuth and BearerAuth. This is now incorrect and makes downstream typing misleading.

def get_auth_pi(use_internal: bool = True) -> Union[HTTPKerberosAuth, BearerAuth]:

tagreader/web_handlers.py:357

• On JSON decode failure this function returns [], but callers treat the result as a dict (e.g., ...webid_dict()[datasource]), which will raise TypeError. Return {} instead. (Also, the function’s return annotation currently indicates a list; it should match the dict returned on the success path.)

    return []

[Copilot]

Pull request overview

Copilot reviewed 4 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (8)

tagreader/web_handlers.py:71

• browser_cookie3.edge(...) returns a CookieJar-like object which does not implement __len__ in the stdlib (len(cookies) will raise TypeError). Convert to a list once and iterate that list so the empty-check is reliable.

    cookies = browser_cookie3.edge(domain_name=".equinor.com")
    if len(cookies) == 0:
        raise ConnectionError(
            "No cookies found for .piwebapi.equinor.com. Please log in to the F5 VPN using Microsoft Edge and try again."
        )

tagreader/web_handlers.py:102

• http.cookiejar.Cookie(...) requires many mandatory constructor arguments; the current call with only name/value/domain/... will raise TypeError. Use requests.cookies.create_cookie(...) (which builds a correct Cookie) before adding it to the jar.

        cookie_jar.set_cookie(
            Cookie(
                name=cookie["name"],
                value=cookie["value"],
                domain=domain,

tagreader/web_handlers.py:192

• This helper prompts via print()/input(), which will block/hang in non-interactive contexts (e.g., services, CI). Prefer logging and raising an actionable exception on timeout so callers can handle the failure instead of waiting indefinitely.

            print("Complete login in the browser window...")

            try:
                page.wait_for_url("**/piwebapi/**", timeout=300_000)
            except PlaywrightTimeoutError:

tagreader/web_handlers.py:199

• get_auth_pi can return a Cookie, CookieJar, or list of cookie dicts in the F5 flow, so the current return annotation (Union[HTTPKerberosAuth, BearerAuth]) is incorrect and breaks type-checking for callers. Widen the return type (or use Any).

def get_auth_pi(use_internal: bool = True) -> Union[HTTPKerberosAuth, BearerAuth]:

tagreader/web_handlers.py:305

• In the cookie-auth branch, session.get(...) currently allows redirects and omits the JSON Accept header, while the non-cookie branch disables redirects and handles 302 as an auth signal. If a redirect occurs, the cookie branch can silently follow it and then fail JSON parsing. Make both branches consistent and check for 302 after the request.

    url_ = urljoin(url, "dataservers")
    if is_cookie_auth(auth):
        session = transfer_browser_context_to_session(cookie_jar=auth)
        res = session.get(url_, verify=verify_ssl, timeout=300)
    else:

tagreader/web_handlers.py:365

• On JSON decode failure this function returns [], but callers expect a mapping (see the successful return {item["Name"]: ...} above). Returning a list will cause runtime errors when indexing by datasource name; return {} instead.

    return []

tagreader/web_handlers.py:380

• self.auth is stored as the original auth argument (possibly None), but later generate_search_params(..., auth=self.auth) passes it into get_piwebapi_source_to_webid_dict(auth=...). When auth wasn’t provided, this causes the dict lookup to use the default auth flow instead of the resolved session auth. Store the resolved auth on self.auth.

        self.auth = auth
        resolved_auth = auth if auth is not None else get_auth_aspen()

examples/demo.py:18

• max_rows is passed as a float (1e7), but this value is used as a row/count limit and should be an int to avoid accidental serialization to a non-integer query parameter.

    handler_options={"max_rows": 1e7},

[Copilot]

Pull request overview

Copilot reviewed 4 out of 6 changed files in this pull request and generated 3 comments.

Suppressed comments (4)

tagreader/web_handlers.py:135

• Playwright represents a session cookie with expires == -1, but Requests interprets that as an already-expired Unix timestamp and will not send it. Normalize -1 to None; otherwise session-scoped F5 cookies returned by the interactive flow cannot authenticate requests.

                    expires=x.get("expires", None),

tagreader/web_handlers.py:203

• The advertised PI app-registration path is not implemented here: use_internal=False returns a user browser cookie (or a list of cookies) and may launch interactive Edge, never a BearerAuth as the annotation and PR title indicate. This cannot support noninteractive app/service-principal authentication. Either implement PI tenant/client/scopes token acquisition or explicitly scope and name this API/PR as interactive F5 cookie authentication.

def get_auth_pi(use_internal: bool = True) -> Union[HTTPKerberosAuth, BearerAuth]:
    if use_internal:
        return HTTPKerberosAuth(mutual_authentication=OPTIONAL)

    cached = _get_cached_f5_auth()

tagreader/web_handlers.py:365

• The successful branch returns a datasource-to-WebId dictionary, and callers index it by datasource name. Returning a list on decode failure violates that contract and turns the later lookup into TypeError; return an empty dictionary instead.

    return []

tagreader/web_handlers.py:110

• The new cookie-authentication path has no automated coverage, while this module's PI handler already has unit and connectivity tests. Add mocked tests for BrowserContext/list conversion (including expires=-1), the no-cookie interactive fallback, cache reuse, and redirect retry so these paths run in CI without requiring Edge or a live PI server.

def transfer_browser_context_to_session(
    cookie_jar: Union[Cookie, CookieJar, BrowserContext, list],
    session: Optional[requests.Session] = None,
    domain_filter: Optional[str] = None,
) -> requests.Session: