Skip to content

Add browser authentication and cytetyper CLI support - #54

Open
Gautam8387 wants to merge 3 commits into
NygenAnalytics:masterfrom
Gautam8387:cli-auth
Open

Add browser authentication and cytetyper CLI support#54
Gautam8387 wants to merge 3 commits into
NygenAnalytics:masterfrom
Gautam8387:cli-auth

Conversation

@Gautam8387

Copy link
Copy Markdown
Member

Description

This PR adds authentication parity between CyteTypeR and the Python client. It introduces browser-based PKCE authentication, shared credential storage, the cytetyper terminal command, authenticated API requests, development URL overrides, and updated documentation and tests.

Summary

  • Add SetupCyteTypeR(), LoginCyteTypeR(), and LogoutCyteTypeR().
  • Add dashboard and job report helpers.
  • Add cytetyper commands for setup, login, logout, dashboard, and job viewing.
  • Share credentials with the Python client through the same local credentials file.
  • Add PKCE, callback state validation, atomic credential writes, and restrictive permissions.
  • Support production defaults and explicit development URLs.
  • Apply authentication to submission, upload, polling, and result retrieval.
  • Add Unix and Windows CLI launchers.
  • Update README, vignettes, rendered guides, and function references.
  • Update package version to 0.4.3.

Authentication flow

sequenceDiagram
    actor User
    participant Client as CyteTypeR or cytetyper
    participant Callback as Local callback server
    participant Browser
    participant API as CyteType API
    participant Store as Shared credentials file

    User->>Client: Start setup
    Client->>Client: Generate PKCE verifier, challenge, and state
    Client->>Callback: Listen on a random 127.0.0.1 port
    Client->>Browser: Open authorization URL
    Browser->>API: Email OTP sign-in and consent
    API-->>Callback: Redirect with authorization code and state
    Callback->>Callback: Validate state
    Callback->>API: Exchange code and PKCE verifier
    API-->>Callback: Return API token and account metadata
    Callback-->>Client: Complete authentication
    Client->>Store: Save credentials atomically
Loading

Client architecture

flowchart LR
    CLI[cytetyper CLI] --> Functions[CyteTypeR authentication functions]
    RClient[CyteTypeR submission functions] --> Resolver[API URL and token resolver]
    Functions --> Resolver
    Resolver <--> Credentials[Shared credentials.json]
    Python[Python CyteType client] <--> Credentials
    Resolver --> API[CyteType API]
    API --> Reports[Dashboard and job reports]
Loading

User-facing commands

SetupCyteTypeR()
LoginCyteTypeR()
LogoutCyteTypeR()
OpenCyteTypeDashboard()
ViewCyteTypeJob("<JOB_ID>")
cytetyper setup
cytetyper login
cytetyper logout
cytetyper dashboard
cytetyper view <JOB_ID>

Development environments can provide their assigned URL without embedding internal URLs:

cytetyper setup --api-url "<URL>"

@qodo-code-review

qodo-code-review Bot commented Aug 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Rate-limit handling regression ✓ Resolved 🐞 Bug ◔ Observability
Description
R/api.R now disables httr2 HTTP error conditions for all non-2xx responses, so
.stop_if_rate_limited() can no longer access e$resp and will not emit the dedicated
RATE_LIMIT_EXCEEDED guidance. Users will instead get a generic HTTP error string for 429 responses,
reducing debuggability and changing prior behavior.
Code

R/api.R[R42-44]

+    response <- req |>
+      httr2::req_error(is_error = function(resp) FALSE) |>
+      req_perform()
Evidence
The new req_error(is_error = function(resp) FALSE) prevents httr2 from emitting httr2_http
errors for HTTP 429, but .stop_if_rate_limited() only triggers when it receives an httr2_http
with e$resp; therefore the specialized RATE_LIMIT_EXCEEDED message can no longer be produced for
.api_response_helper() calls.

R/api.R[42-45]
R/api.R[69-76]
R/api.R[91-96]
R/errors.R[36-47]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`R/api.R:.api_response_helper()` now uses `httr2::req_error(is_error = function(resp) FALSE)`, preventing `httr2` from raising `httr2_http` conditions on HTTP failures (including 429). The existing rate-limit handler `.stop_if_rate_limited()` only triggers when it catches an `httr2_http` with `e$resp`, so throttling responses lose the custom “RATE_LIMIT_EXCEEDED” guidance.

### Issue Context
- `.api_response_helper()` still calls `.stop_if_rate_limited(e)` in its `tryCatch` error handler, but for HTTP 429 there is no thrown `httr2_http` anymore—only a normal response object followed by `stop("HTTP ...")`.
- The repo already has a dedicated rate-limit UX in `R/errors.R:.stop_if_rate_limited()`; the goal is to preserve that behavior.

### Fix Focus Areas
- R/api.R[42-76]
- R/errors.R[36-47]

### Suggested implementation direction
Pick one:
1) **Call the rate-limit logic before `stop()`** inside the non-2xx branch in `.api_response_helper()`:
  - After `parsed <- .parse_server_error(response)`, if `parsed$error_code == "RATE_LIMIT_EXCEEDED"`, raise the same message currently produced by `.stop_if_rate_limited()` (including the “Use your own LLM API key…” guidance).

2) **Stop disabling httr2 errors for 429**:
  - Change `req_error(is_error=...)` to return `TRUE` for statuses you want httr2 to throw (e.g., 429), but keep returning `FALSE` for statuses you intentionally handle as normal responses (401/403/404). Then update `.api_response_helper()` to catch `httr2_http` and inspect `e$resp` similarly to the previous behavior.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread R/api.R
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add browser-based PKCE authentication and cytetyper CLI to CyteTypeR

✨ Enhancement 📝 Documentation 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Introduces browser-based PKCE sign-in and shared credential storage with the Python client.
• Adds the cytetyper terminal CLI plus Unix/Windows launchers and an installer function.
• Applies authenticated requests across submission, uploads, polling, and result retrieval.
• Adds secure, atomic credential writes with strict permissions and state validation.
• Updates defaults (CYTETYPE_API_URL support), docs/vignettes, and expands test coverage.
Diagram

graph TD
  CLI["cytetyper CLI"] --> AuthMod["Auth functions"] --> Callback["Callback server"] --> API{{"CyteType API"}}
  AuthMod --> Creds[("credentials.json")]
  Client["CyteTypeR client"] --> Resolver["Token/URL resolver"] --> Creds --> API
  Python{{"Python client"}} --> Creds
  subgraph Legend
    direction LR
    _mod["Module"] ~~~ _db[("File")] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

Using a loopback redirect + PKCE is a standard, secure pattern for CLI/desktop auth, and a shared credentials file is the simplest way to achieve parity with the Python client without introducing OS-specific keychain integrations or a background credential service. Device-code flow or platform keychains were plausible alternatives but would increase platform complexity or require additional server-side support.

Files changed (33) +3819 / -242

Enhancement (8) +1696 / -24
NAMESPACEExport authentication and CLI entrypoints +6/-0

Export authentication and CLI entrypoints

• Exports SetupCyteTypeR/LoginCyteTypeR/LogoutCyteTypeR, dashboard/job helpers, and InstallCyteTypeRCli so users can authenticate and install/use the CLI.

NAMESPACE

api.RPreserve 401/403 as typed authentication errors +11/-3

Preserve 401/403 as typed authentication errors

• Disables httr2 default error raising for API calls and maps 401/403 responses into .stop_authentication() errors (cytetype_auth_error). Improves error propagation for auth failures.

R/api.R

auth.RImplement PKCE browser auth and secure credential storage +1039/-0

Implement PKCE browser auth and secure credential storage

• Adds the full auth subsystem: API URL validation, shared credentials path resolution, atomic credential writes with strict permissions, PKCE + state generation/validation, a local httpuv callback app, and helper URLs for dashboard/job viewing. Introduces exported user functions for setup/login/logout and opening dashboard/job URLs.

R/auth.R

cli.RAdd cytetyper CLI dispatcher plus installer +351/-0

Add cytetyper CLI dispatcher plus installer

• Implements cytetyper command parsing and dispatch (setup/get-key/login/logout/dashboard/view and --version). Adds InstallCyteTypeRCli() to install an OS-appropriate launcher alongside the active Rscript.

R/cli.R

cytetype.RResolve auth token/URL and strengthen GetResults origin handling +78/-21

Resolve auth token/URL and strengthen GetResults origin handling

• Uses .resolve_api_url() for consistent origin handling and .resolve_auth_token() to require credentials by default for authenticated endpoints. Extends GetResults() with api_url support and enforces origin matching when an explicit auth_token is supplied for stored jobs.

R/cytetype.R

cytetyperAdd Unix cytetyper launcher script +27/-0

Add Unix cytetyper launcher script

• Adds a POSIX shell launcher that runs CyteTypeR’s internal CLI entrypoint with the adjacent Rscript, forwarding args and exit code.

exec/cytetyper

cytetyper.cmdAdd Windows cytetyper launcher script +4/-0

Add Windows cytetyper launcher script

• Adds a Windows .cmd launcher that invokes Rscript.exe and calls the internal CLI entrypoint, matching Unix behavior.

exec/cytetyper.cmd

cli_callback.htmlAdd callback HTML template for browser auth completion +180/-0

Add callback HTML template for browser auth completion

• Provides the local callback landing page with placeholders for status/message/email/dashboard URL and a timed redirect. Designed to avoid token/code disclosure and to apply strict security headers.

inst/templates/cli_callback.html

Bug fix (1) +6 / -0
client.RPropagate auth errors through status/results requests +6/-0

Propagate auth errors through status/results requests

• Ensures cytetype_auth_error conditions are rethrown from .make_results_request() rather than being converted into generic failed/error statuses.

R/client.R

Refactor (1) +2 / -1
seurat_helpers.RStore normalized, validated API origins in Seurat job details +2/-1

Store normalized, validated API origins in Seurat job details

• Validates api_url and constructs report URLs via .url_path() (avoids Windows backslashes) before persisting job details to obj@misc.

R/seurat_helpers.R

Documentation (15) +742 / -206
README.mdAdd authentication + cytetyper CLI documentation +74/-15

Add authentication + cytetyper CLI documentation

• Documents browser sign-in, manual API-key login, cytetyper installation/usage, and development URL overrides. Updates example report URLs to the production domain.

README.md

configurations.mdDocument API URL precedence and auth workflows +93/-23

Document API URL precedence and auth workflows

• Restructures the configuration guide to cover API origin resolution, development environment guidance, authentication functions/CLI commands, and LLM credential hygiene.

docs/configurations.md

examples.mdUpdate example report links to production domain +20/-20

Update example report links to production domain

• Replaces prod.cytetype.nygen.io links with cytetype.nygen.io across example tables.

docs/examples.md

get-started.mdAdd sign-in instructions and CLI usage to get-started +117/-66

Add sign-in instructions and CLI usage to get-started

• Adds an authentication section, updates the quick-start workflow examples, and documents opening dashboards/job reports from both R and the CLI.

docs/get-started.md

CyteTypeR.RdRegenerate CyteTypeR docs for auth resolution behavior +15/-8

Regenerate CyteTypeR docs for auth resolution behavior

• Updates generated documentation to describe api_url precedence and auth_token defaulting to saved credentials, plus save_query guidance.

man/CyteTypeR.Rd

GetResults.RdRegenerate GetResults docs with api_url support +12/-4

Regenerate GetResults docs with api_url support

• Adds api_url argument documentation and clarifies how saved credentials vs explicit tokens are used when fetching stored jobs.

man/GetResults.Rd

InstallCyteTypeRCli.RdAdd generated docs for InstallCyteTypeRCli +31/-0

Add generated docs for InstallCyteTypeRCli

• Introduces the man page describing how to install and use the cytetyper launcher.

man/InstallCyteTypeRCli.Rd

LoginCyteTypeR.RdAdd generated docs for LoginCyteTypeR +26/-0

Add generated docs for LoginCyteTypeR

• Introduces the man page describing validating and saving an existing API key via a hidden prompt.

man/LoginCyteTypeR.Rd

LogoutCyteTypeR.RdAdd generated docs for LogoutCyteTypeR +23/-0

Add generated docs for LogoutCyteTypeR

• Introduces the man page describing removal of the local credentials file and server-side revocation guidance.

man/LogoutCyteTypeR.Rd

OpenCyteTypeDashboard.RdAdd generated docs for OpenCyteTypeDashboard +23/-0

Add generated docs for OpenCyteTypeDashboard

• Introduces the man page describing dashboard URL selection and browser opening behavior.

man/OpenCyteTypeDashboard.Rd

PrepareCyteTypeR.RdClarify marker filtering and subsampling docs +4/-2

Clarify marker filtering and subsampling docs

• Updates generated documentation to clarify n_top_genes behavior and max_cells_per_group usage for visualization subsampling.

man/PrepareCyteTypeR.Rd

SetupCyteTypeR.RdAdd generated docs for SetupCyteTypeR +31/-0

Add generated docs for SetupCyteTypeR

• Introduces the man page for the browser-based PKCE setup flow and shared credential file behavior.

man/SetupCyteTypeR.Rd

ViewCyteTypeJob.RdAdd generated docs for ViewCyteTypeJob +26/-0

Add generated docs for ViewCyteTypeJob

• Introduces the man page describing construction of login redirect URLs for job reports.

man/ViewCyteTypeJob.Rd

configurations.RmdUpdate configurations vignette for auth + API URL +110/-21

Update configurations vignette for auth + API URL

• Aligns vignette content with new API URL precedence rules, authentication flows, and LLM credential guidance; switches to dynamic date.

vignettes/configurations.Rmd

get-started.RmdUpdate get-started vignette with authentication and CLI +137/-47

Update get-started vignette with authentication and CLI

• Adds sections for SetupCyteTypeR/LoginCyteTypeR and cytetyper usage, updates examples, and documents development URL overrides.

vignettes/get-started.Rmd

Other (8) +1373 / -11
DESCRIPTIONBump version and add auth dependencies +5/-2

Bump version and add auth dependencies

• Adds askpass, httpuv, and openssl imports required for browser login, local callback serving, and PKCE. Bumps package version to 0.4.3 and updates roxygen config metadata.

DESCRIPTION

zzz.RUpdate default API URL and support CYTETYPE_API_URL overrides +14/-7

Update default API URL and support CYTETYPE_API_URL overrides

• Stops overwriting user-supplied options on load, changes the default origin to https://cytetype.nygen.io, and adds precedence for the CYTETYPE_API_URL environment variable.

R/zzz.R

test-api.RAdd test for typed auth failures from API helper +21/-0

Add test for typed auth failures from API helper

• Adds a test ensuring .api_response_helper() raises cytetype_auth_error on 401 with an informative message.

tests/testthat/test-api.R

test-auth.RAdd comprehensive auth subsystem tests +939/-0

Add comprehensive auth subsystem tests

• Adds tests for URL validation, secure credential save/load/delete, PKCE/state generation, callback state validation, HTML escaping, browser launching (including WSL), and credential-aware GetResults behavior.

tests/testthat/test-auth.R

test-cli.RAdd cytetyper CLI parsing/routing/installer tests +188/-0

Add cytetyper CLI parsing/routing/installer tests

• Tests command routing, --api-url parsing forms, help/version behavior, parse errors, launcher selection, and InstallCyteTypeRCli installation rules.

tests/testthat/test-cli.R

test-client.REnsure result requests preserve authentication errors +22/-0

Ensure result requests preserve authentication errors

• Adds a unit test confirming .make_results_request() rethrows cytetype_auth_error conditions.

tests/testthat/test-client.R

test-cytetype-build-upload.RCover auth behavior in build/upload/submission workflow +145/-1

Cover auth behavior in build/upload/submission workflow

• Updates fixtures and adds tests ensuring missing credentials fail early, stored credentials are used when auth_token is NULL, explicit tokens override stored credentials, and completed workflows store results retrievable via GetResults().

tests/testthat/test-cytetype-build-upload.R

test-defaults.RTest environment-variable precedence for API URL defaults +39/-1

Test environment-variable precedence for API URL defaults

• Updates default URL expectation to cytetype.nygen.io and adds coverage for CYTETYPE_API_URL taking precedence over option defaults.

tests/testthat/test-defaults.R

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant