Add authorization request store

[thiva-k]

Purpose

This pull request introduces a new mechanism for storing OAuth2 authorization request context in the backend, replacing the previous session-based approach. The changes ensure that authorization request state is managed securely and efficiently using a dedicated database table and store interface. The update affects both the database schema and the handler logic, providing better scalability and maintainability for the OAuth2 authorization flow.

Approach

Database schema changes:

• Added a new AUTHORIZATION_REQUEST table and corresponding expiry index to both postgres.sql and sqlite.sql for storing OAuth2 authorization request context, including request parameters and timestamps. [1] [2]

Backend logic and handler refactoring:

• Introduced the authorizationRequestStore implementation (auth_req_store.go) with methods for adding, retrieving, and clearing authorization request contexts, using the new database table.
• Refactored the authorizeHandler to use the new authorizationRequestStoreInterface instead of the previous session store, updating constructor, field names, and method calls throughout. [1] [2]
• Replaced session data handling in the authorization flow with the new request context, including storing, retrieving, and passing the context between requests and responses. [1] [2] [3] [4] [5]
• Updated request/response parameter names and logic to use AuthRequestID instead of SessionDataKey in relevant handler methods and request parsing. [1] [2]

These changes collectively improve the reliability and clarity of the OAuth2 authorization flow by decoupling request state from session management and persisting it in a dedicated, queryable store.

Related issue

• OAuth Package Refactoring and Improvements #550

[Copilot]

Pull request overview

This PR migrates OAuth2 authorization request context storage from an in-memory session-based approach to a persistent database-backed solution. The change improves scalability, reliability, and state management for OAuth2 authorization flows by introducing a dedicated AUTHORIZATION_REQUEST table and corresponding store interface.

Key Changes

• Added AUTHORIZATION_REQUEST table to both SQLite and PostgreSQL schemas with proper indexing for expiry-based queries
• Implemented authorizationRequestStore with database persistence replacing the previous in-memory sessionDataStore
• Refactored handlers to use AuthRequestID instead of SessionDataKey for better naming clarity

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
backend/dbscripts/runtimedb/sqlite.sql	Added AUTHORIZATION_REQUEST table with TEXT storage for request data
backend/dbscripts/runtimedb/postgres.sql	Added AUTHORIZATION_REQUEST table with JSONB storage for request data
tests/integration/resources/dbscripts/runtimedb/sqlite.sql	Integration test schema with matching table structure
tests/integration/resources/dbscripts/runtimedb/postgres.sql	Integration test schema with matching table structure
backend/internal/oauth/oauth2/authz/auth_req_store.go	New database-backed store implementation with JSON serialization
backend/internal/oauth/oauth2/authz/auth_req_store_test.go	Comprehensive unit tests for the new store (837 lines)
backend/internal/oauth/oauth2/authz/auth_code_store.go	New authorization code store implementation
backend/internal/oauth/oauth2/authz/auth_code_store_test.go	Unit tests for authorization code store (681 lines)
backend/internal/oauth/oauth2/authz/store_constants.go	Added database queries for authorization request operations
backend/internal/oauth/oauth2/authz/handler.go	Refactored to use new store and renamed fields from SessionDataKey to AuthRequestID
backend/internal/oauth/oauth2/authz/handler_test.go	Updated tests to use new store and naming
backend/internal/oauth/oauth2/authz/init.go	Added authorization request store initialization
backend/internal/oauth/oauth2/authz/model.go	Updated model with AuthRequestID instead of SessionDataKey
backend/internal/oauth/oauth2/constants/constants.go	Renamed constant from SessionDataKey to AuthRequestID
backend/internal/observability/event/datakeys.go	Updated event data keys to use AuthRequestID
backend/internal/oauth/oauth2/authz/session_store.go	Removed old in-memory session store
backend/internal/oauth/oauth2/authz/session_store_test.go	Removed old session store tests
backend/internal/oauth/oauth2/authz/sessionDataStoreInterface_mock_test.go	Removed old mock
backend/tests/mocks/oauth/oauth2/authzmock/sessionDataStoreInterface_mock.go	Removed old mock

[Copilot]

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

backend/internal/oauth/oauth2/authz/handler.go:152

• Unencoded user-controlled state appended to redirect URL using string concatenation. This enables query-parameter injection (e.g., adding &foo=bar) into the client application's redirect URI. Encode state or build the URL via a safe utility (e.g., GetURIWithQueryParams).

Severity: MEDIUM. Confidence: 9

				redirectURI += "&" + oauth2const.RequestParamState + "=" + state

[codecov]

Codecov Report

❌ Patch coverage is 86.95652% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.63%. Comparing base (7311ba5) to head (2ca1373).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
backend/internal/oauth/oauth2/authz/handler.go	78.88%	16 Missing and 3 partials ⚠️
...kend/internal/oauth/oauth2/authz/auth_req_store.go	90.59%	5 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #873      +/-   ##
==========================================
- Coverage   87.69%   87.63%   -0.06%     
==========================================
  Files         318      319       +1     
  Lines       26152    26261     +109     
  Branches      606      606              
==========================================
+ Hits        22934    23014      +80     
- Misses       2002     2023      +21     
- Partials     1216     1224       +8     

Flag	Coverage Δ
backend-integration-postgres	58.71% <55.21%> (-0.04%)	⬇️
backend-integration-sqlite	58.68% <55.21%> (-0.04%)	⬇️
backend-unit	74.90% <86.95%> (-0.01%)	⬇️
frontend-apps-develop-unit	88.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.

[senthalan]

shouldn't this be read from the configuration?

If the spec mandates 10 mins as the expiry time, then hardcoding it here is fine. If not, we need let it configurable

[ThaminduDilshan]

Since this is a existing implementation, shall we introduce a config in a separate PR?

[senthalan]

why we are taking the assertion authTime as the created time for the auth code?

Is it a spec recommendation

[thiva-k]

This created time becomes the auth_time in the ID token, which should ideally be the time the assertion was created right?

Should we have both TimeCreated and AuthTime fields in Authorization code context ?

[senthalan]

oh. I thought we are using it for access token. If it's been used only for the auth_time in the ID token, then it should be fine

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.