Skip to content

fix: harden dangling OAuth policy handling and error classification#204

Open
sergey3bv wants to merge 1 commit into
divinevideo:mainfrom
sergey3bv:chore/policy-enforcment
Open

fix: harden dangling OAuth policy handling and error classification#204
sergey3bv wants to merge 1 commit into
divinevideo:mainfrom
sergey3bv:chore/policy-enforcment

Conversation

@sergey3bv
Copy link
Copy Markdown
Contributor

Summary

  • fail-closed handling for missing policy_id;
  • migration cleanup to make FK addition safe;
  • regression test coverage update.

Avoid naming corporate partners, customers, or other sensitive external brands in this PR title or body. Use generic descriptors unless a maintainer explicitly approves the public reference.

Motivation

Related Issue

Testing

  • cargo test --workspace --verbose
  • cargo clippy --workspace --all-targets --all-features -- -D warnings -A deprecated
  • cargo fmt --all -- --check
  • Manual verification completed

Visuals

  • UI/web change with screenshots/video attached
  • No visual change
  • Visuals and text avoid sensitive external brand or partner names unless explicitly approved

Copy link
Copy Markdown
Contributor

@dcadenas dcadenas left a comment

Choose a reason for hiding this comment

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

The migration still lets a revoked OAuth row reach permission checks as unrestricted access. Fix the active-authorization checks before merge so clearing policy_id to NULL never becomes the full-access path for revoked rows.

WHEN oa.revoked_at IS NULL THEN NOW()
ELSE oa.revoked_at
END,
policy_id = NULL,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Keep revoked OAuth rows out before any NULL policy branch can allow access.
This migration sets revoked_at and clears dangling policy_id to NULL, but the later origin lookup and signer reload can still evaluate that row as unrestricted access.
Add revoked_at IS NULL to the active OAuth authorization lookup and make signer permission validation reject any refetched OAuth authorization with revoked_at set.
Please cover both the origin-based HTTP validator and signer validation for a revoked policy_id = NULL row, while preserving active NULL-policy access and valid empty-policy access.

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.

chore(auth): decide policy enforcement for missing or dangling oauth_authorizations.policy_id

2 participants