Skip to content

Conversation

@tolgaozen
Copy link
Member

@tolgaozen tolgaozen commented Oct 7, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined authentication flow with context-aware handling and clearer, more consistent logging and errors.
  • Performance
    • Reduced latency and contention in key retrieval and JWKS refresh paths.
  • Migrations
    • Added/updated concurrent indexes (including tenant-scoped composites) to improve query performance.
    • Uses non-transactional concurrent operations to minimize lock time and upgrade impact.
  • Chores
    • Improved migration scripts with clearer structure and annotations for safer rollouts and rollbacks.

@coderabbitai
Copy link

coderabbitai bot commented Oct 7, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Refactors internal OpenID authentication to use context-aware flows for token processing and JWKS key retrieval, with refined retry/backoff handling and expanded logging. Updates two PostgreSQL migration scripts to add explicit NO TRANSACTION guidance, comments, and clarified Up/Down structure without changing effective index operations.

Changes

Cohort / File(s) Summary
OIDC authn refactor
internal/authn/openid/authn.go
Switched methods to accept/use ctx; adjusted key retrieval retry/backoff, JWKS fetch/refresh concurrency, and error propagation; expanded logging/comments; no exported API changes.
Schema version index migration
internal/storage/postgres/migrations/20231230174418_schema_version_index.sql
Rewrote migration with explicit NO TRANSACTION notes and comments; preserved CREATE/DROP INDEX CONCURRENTLY logic; clarified Up/Down sections.
Tenant-scoped indexes migration
internal/storage/postgres/migrations/20250912101325_add_tenant_scoped_indexes.sql
Added comments and NO TRANSACTION markers; maintained functional sequence: create composite tenant-scoped indexes, drop old single-column index; mirrored in Down.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Authn as Authn.Authenticate
  participant Cache as Key Cache
  participant JWKS as JWKS Manager
  participant HTTP as HTTP Client
  participant OIDC as OIDC Provider

  Client->>Authn: Authenticate(ctx, token)
  Authn->>Cache: Get(keyID)
  alt Key hit
    Cache-->>Authn: Public Key
    Authn-->>Client: Verify token and return result
  else Key miss
    Authn->>JWKS: getKeyWithRetry(ctx, keyID)
    rect rgba(200,230,255,0.3)
      note right of JWKS: Retry/backoff loop with ctx cancellation
      JWKS->>Cache: Check/refresh state (single-refresh)
      opt Needs refresh
        JWKS->>HTTP: GET /.well-known/openid-configuration
        HTTP-->>JWKS: OIDC config
        JWKS->>HTTP: GET jwks_uri
        HTTP-->>JWKS: JWKS document
        JWKS->>Cache: Update keys
      end
      JWKS-->>Authn: Key or error
    end
    alt Key found
      Authn-->>Client: Verify token and return result
    else Error
      Authn-->>Client: Context-aware error
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

I twitch my ears at ctx winds,
JWKS keys where daylight grins.
Indices dance, CONCURRENTLY so,
Up and Down in tidy flow.
With careful hops through backoff dew,
I sign, I fetch—then hop anew. 🐇✨

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/authn-openid-refactor

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60ce39f and 1642d1a.

📒 Files selected for processing (3)
  • internal/authn/openid/authn.go (13 hunks)
  • internal/storage/postgres/migrations/20231230174418_schema_version_index.sql (1 hunks)
  • internal/storage/postgres/migrations/20250912101325_add_tenant_scoped_indexes.sql (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tolgaozen tolgaozen merged commit 9a7ce52 into master Oct 7, 2025
8 of 10 checks passed
@tolgaozen tolgaozen deleted the feature/authn-openid-refactor branch October 7, 2025 15:19
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