Skip to content

Add CLI authentication and automatic SDK credential loading - #90

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

Add CLI authentication and automatic SDK credential loading#90
Gautam8387 wants to merge 4 commits into
NygenAnalytics:masterfrom
Gautam8387:cli-auth

Conversation

@Gautam8387

Copy link
Copy Markdown
Member

Summary

This PR adds a packaged cytetype command-line interface and connects it to the existing Python SDK.

Users can authenticate once through their browser with cytetype setup. The resulting API key is stored in an owner-only local credential file and automatically used by subsequent SDK annotation and upload requests.

Users can also save an existing API key using cytetype login.

CLI commands

  • cytetype setup

    • Starts a PKCE browser authorization flow.
    • Opens the configured CyteType login page.
    • Prints the authorization URL when the browser cannot be opened.
    • Receives the authorization result through a local callback.
    • Saves the returned credentials.
    • Shows a branded success page and redirects to the dashboard after five seconds.
  • cytetype get-key

    • Alias for cytetype setup.
  • cytetype login

    • Prompts for an existing API key using hidden terminal input.
    • Validates the key with the server.
    • Retrieves and stores the associated user metadata.
  • cytetype dashboard

    • Prints and opens the dashboard associated with the saved server.
  • cytetype view JOB_ID

    • Prints and opens the web report for a job.
    • Does not place the API key in the URL.
  • cytetype logout

    • Deletes local credentials.
    • Does not revoke the server-side API key.
  • cytetype --version

    • Prints the installed CyteType version.

Credential storage

Credentials are stored in:

  • $XDG_CONFIG_HOME/cytetype/credentials.json when configured.
  • %APPDATA%/cytetype/credentials.json on Windows.
  • ~/.config/cytetype/credentials.json by default.

The credential file contains:

  • API URL
  • Dashboard URL
  • API key
  • Token ID
  • User ID
  • Email

Storage behavior:

  • Parent directory is restricted to the current user.
  • Credential file mode is set to 0600.
  • Writes use a temporary file followed by an atomic replacement.
  • Credentials are bound to their saved API URL.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add CLI authentication (cytetype setup/login) and SDK auto credential loading

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Adds a packaged cytetype CLI with setup, get-key, login, dashboard, view, logout,
 and --version commands.
• Implements a PKCE browser-based auth flow with a local HTTP callback server and a branded HTML
 success/error page.
• Stores credentials in an owner-only (0600) local file, keyed to the API URL, using atomic writes.
• SDK (CyteType) now auto-loads the stored API key per API URL when no explicit auth_token is
 given, and defaults api_url to CYTETYPE_API_URL env var.
• Adds extensive unit tests for CLI flows, credential storage, and SDK credential-resolution
 behavior.
Diagram

sequenceDiagram
    actor User
    participant CLI as cytetype CLI
    participant Browser
    participant API as CyteType API
    participant Store as Credentials File
    participant SDK as CyteType SDK

    User->>CLI: cytetype setup
    CLI->>Browser: Open authorize URL (PKCE)
    Browser->>API: Authorize request
    API-->>CLI: Callback with code
    CLI->>API: Exchange code for token
    API-->>CLI: apiToken, tokenId, userId
    CLI->>Store: Save credentials (0600)
    User->>SDK: CyteType.run()
    SDK->>Store: Load credentials by apiUrl
    SDK->>API: Annotation request with token
Loading
High-Level Assessment

A local-server PKCE flow with atomic, permission-restricted credential storage is the standard, secure approach for CLI OAuth-style authentication (mirrors patterns used by gh, aws, gcloud CLIs). No meaningfully better alternative (e.g., device-code flow) was needed given the existing browser-based login page; the chosen approach minimizes server-side changes while keeping the API key out of URLs and terminal history.

Files changed (9) +1507 / -34

Enhancement (4) +842 / -29
cli.pyNew CLI entrypoint implementing setup/login/dashboard/view/logout +502/-0

New CLI entrypoint implementing setup/login/dashboard/view/logout

• Adds a new module implementing the 'cytetype' CLI: argparse-based command dispatch, a PKCE browser auth flow with a local HTTP callback server, browser-launch helpers (including WSL handling), and login/dashboard/view/logout commands.

cytetype/cli.py

config.pyAdd credential storage, loading, and API URL helpers +86/-2

Add credential storage, loading, and API URL helpers

• Introduces 'StoredCredentials' pydantic model, cross-platform credential path resolution (XDG/APPDATA/home), atomic save with 0600 permissions, load/delete helpers, and default API URL resolution via 'CYTETYPE_API_URL'.

cytetype/config.py

main.pySDK auto-loads stored credentials per API URL +74/-27

SDK auto-loads stored credentials per API URL

• Reworks imports, defaults 'api_url' to env-configured value, and adds '_resolve_auth_token' to transparently load and cache stored CLI credentials scoped to the target API URL for 'run()' and 'get_results()'.

cytetype/main.py

cli_callback.htmlAdd branded HTML success/error page for OAuth callback +180/-0

Add branded HTML success/error page for OAuth callback

• New static HTML/CSS template rendered by the local callback server showing success/error state, dashboard redirect, and countdown animation.

cytetype/templates/cli_callback.html

Tests (2) +657 / -4
test_cli.pyAdd comprehensive CLI and credential storage tests +534/-0

Add comprehensive CLI and credential storage tests

• New test suite covering credential round-trip/permissions, PKCE setup flow (success, timeout, wrong state), login, dashboard/view/logout commands, browser launcher behavior (including WSL), and API URL validation.

tests/test_cli.py

test_cytetype_integration.pyAdd tests for SDK env-based API URL and stored-credential auth resolution +123/-4

Add tests for SDK env-based API URL and stored-credential auth resolution

• Adds tests verifying 'CYTETYPE_API_URL' env default, explicit override, automatic credential loading per API URL, per-server credential isolation, and error when no credentials/setup exist.

tests/test_cytetype_integration.py

Other (3) +8 / -1
__init__.pyBump package version to 0.19.5 +1/-1

Bump package version to 0.19.5

• Version bump accompanying the new CLI feature release.

cytetype/init.py

pyproject.tomlRegister 'cytetype' console script and package HTML templates +6/-0

Register 'cytetype' console script and package HTML templates

• Adds '[project.scripts]' entry point wiring 'cytetype' to 'cytetype.cli:main' and includes template HTML files as package data.

pyproject.toml

.gitignoreIgnore local tmp/ directory +1/-0

Ignore local tmp/ directory

• Adds 'tmp/' to gitignore.

.gitignore

@qodo-code-review

qodo-code-review Bot commented Aug 5, 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


Action required

1. Unterminated authorize_url f-string ✓ Resolved 🐞 Bug ≡ Correctness
Description
In cytetype.cli._run_setup(), authorize_url is built with a newline inside a non-triple-quoted
f-string literal, which makes cytetype/cli.py fail to import with a SyntaxError and breaks the
installed cytetype console script.
Code

cytetype/cli.py[R322-323]

+        authorize_url = f"{api_url}/auth/cli/authorize?{
+            urlencode(
Evidence
The f-string starts with a double quote on line 322 and then immediately hits a newline before the
quote is closed, which is invalid syntax and prevents module import.

cytetype/cli.py[321-331]

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

### Issue description
`authorize_url` is currently constructed using an f-string that spans multiple lines without triple quotes. This leaves the string literal unterminated and causes a SyntaxError at import time, so the CLI entrypoint (`cytetype = cytetype.cli:main`) cannot run.

### Issue Context
The problematic code is in `_run_setup()` when assembling the authorization URL.

### Fix Focus Areas
- cytetype/cli.py[321-331]

### Suggested fix
Compute the query separately and keep the f-string on one line, e.g.:

```py
params = urlencode({
   "redirectUri": redirect_uri,
   "state": state,
   "codeChallenge": challenge,
})
authorize_url = f"{api_url}/auth/cli/authorize?{params}"
```

(or use explicit string concatenation across lines).

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



Remediation recommended

2. Credentials dir permissions unchecked ✓ Resolved 🐞 Bug ⛨ Security
Description
save_credentials() only applies mode=0700 when creating the credentials directory; if the directory
already exists with permissive permissions, it is reused as-is, allowing other local users (in
misconfigured/shared environments) to delete/replace credentials.json even though the file itself is
chmod’d to 0600.
Code

cytetype/config.py[R48-50]

+    path = get_credentials_path()
+    path.parent.mkdir(mode=0o700, parents=True, exist_ok=True)
+    descriptor, temporary_name = tempfile.mkstemp(
Evidence
The code creates the directory with mode=0700 only on creation, but does not re-check or correct
permissions when it already exists; only the file is chmod’d to 0600 afterward.

cytetype/config.py[47-63]

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

### Issue description
`save_credentials()` intends to store secrets under a user-private directory, but `mkdir(..., exist_ok=True)` does not correct permissions on an existing directory. If that directory is group/world-writable, a local attacker could tamper with `credentials.json` (delete/replace/symlink) despite the file being `0600`.

### Issue Context
This risk exists specifically when `~/.config/cytetype` (or the configured XDG/APPDATA path) already exists with unsafe permissions.

### Fix Focus Areas
- cytetype/config.py[47-67]

### Suggested fix
After ensuring the directory exists:
- On POSIX, stat `path.parent` and either:
 - `chmod(0o700)` if group/other bits are present, or
 - raise a clear error if permissions/ownership are unsafe.
- Consider verifying directory ownership (uid) on POSIX before writing.
- On Windows, skip POSIX mode checks and rely on ACLs (or add a best-effort warning).

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


To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread cytetype/cli.py Outdated
Comment thread cytetype/config.py
@parashardhapola

Copy link
Copy Markdown
Member

@Gautam8387

get_results() trusts stored api_url and may send an explicit token to it. A modified AnnData file could therefore redirect the token.

Bind every token to the exact API URL where it was supplied.

# During initialization
self._auth_token_api_url = self.api_url if auth_token else None

Then change token resolution so:

  1. A newly supplied token is bound to the current API URL.
  2. A cached token is reused only when its bound URL matches.
  3. On mismatch, load credentials specifically for the target URL.
  4. If none exist, raise before any network request.
if auth_token:
    self.auth_token = auth_token
    self._auth_token_api_url = normalized_api_url
    return auth_token

if self.auth_token and self._auth_token_api_url == normalized_api_url:
    return self.auth_token

Keep the API URL in AnnData so old jobs can be retrieved, but treat it as untrusted. Validate it and never pair it with a token bound to another URL.

Add tests proving that a modified job URL raises and that no upload, submission, polling, or result request occurs

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.

2 participants