Skip to content

PR 11 — Auth hardening phase 1: unified security module - #45

Merged
AlexKitipov merged 1 commit into
mainfrom
codex/consolidate-authentication-security-module
Jun 9, 2026
Merged

PR 11 — Auth hardening phase 1: unified security module#45
AlexKitipov merged 1 commit into
mainfrom
codex/consolidate-authentication-security-module

Conversation

@AlexKitipov

Copy link
Copy Markdown
Owner

Motivation

  • Consolidate duplicated JWT and password helpers into a single canonical module to avoid divergence and make auth behavior predictable.
  • Normalize token subjects so sub is stored as a string in tokens and converted to an integer only at the authentication boundary to centralize validation.
  • Ensure all protected endpoints consume a consistent dependency that returns the active User object so authorization checks live at the same boundary.

Description

  • Moved and hardened token/password logic to app/security.py, adding _token_expiration, string-normalization of sub in create_token, a robust verify_token, and a boundary helper token_subject_as_user_id that parses/validates the subject to an integer.
  • Replaced duplicated utilities in app/utils/security.py with compatibility wrappers that delegate to the canonical app.security helpers (create_access_token, create_refresh_token, etc.).
  • Implemented get_current_user in app/dependencies.py which uses verify_token and token_subject_as_user_id and returns an active User instance or raises a 401.
  • Updated v1 endpoints (auth, links, analytics, users) to depend on get_current_user and accept current_user: User (instead of raw user_id), and updated refresh flow to parse the refresh token subject via token_subject_as_user_id.
  • Added tests/test_security.py exercising subject normalization, token type enforcement, invalid subject handling, and the compatibility wrappers.

Testing

  • Ran the test suite with pytest -q which executed the application tests including the new tests/test_security.py and completed successfully (13 passed).
  • Tests passed but emitted dependency/deprecation warnings from third-party libraries related to naive utcnow() usage; no test failures occurred.
  • Formatting was applied with black/ruff during the change and all automated tests continued to pass after refactors.

Codex Task

@AlexKitipov
AlexKitipov merged commit a1ca528 into main Jun 9, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0653da939

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/utils/security.py
hash_password,
token_subject_as_user_id,
verify_password,
verify_token,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the legacy utils verifier contract

Because this module is still presented as a compatibility layer, directly re-exporting the canonical verify_token changes the old app.utils.security.verify_token behavior: it now defaults to requiring an access-token type claim and returns the raw payload with sub, whereas the previous helper accepted tokens without an access type and returned {"user_id": ...}. Any existing code that kept importing this compatibility path and reads payload["user_id"] (or verifies refresh tokens from create_refresh_token() without passing a token type) will start failing at runtime; keep a small wrapper here if the old import path is meant to remain compatible.

Useful? React with 👍 / 👎.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant