Skip to content

Improvements on browser SSO - #3907

Merged
madurangasiriwardena merged 1 commit into
thunder-id:mainfrom
madurangasiriwardena:feature/session-poc-followup
Jul 10, 2026
Merged

Improvements on browser SSO#3907
madurangasiriwardena merged 1 commit into
thunder-id:mainfrom
madurangasiriwardena:feature/session-poc-followup

Conversation

@madurangasiriwardena

@madurangasiriwardena madurangasiriwardena commented Jul 10, 2026

Copy link
Copy Markdown
Member

Purpose

Follow-up to #3779 (flow-centric browser SSO), addressing review comments raised after merge. Pure code-organization and internal-cleanup changes in the flow session subsystem — no behavioral or API changes.

Approach

  • Single store implementation — now that the SSO session, session-context, and participant stores share one store type behind a single package-private sessionStore interface, their split source files (session_context_store.go, participant_store.go) are merged into store.go. All methods already had the *store receiver, so this is a pure file consolidation.
  • Uniform query IDs — the SQL query constants are renumbered under one SSO-SESS-NN prefix (01–11), dropping the old -AC-/-PART- sub-prefixes that reflected the previously-separate stores. IDs are internal identifiers only (no DB/functional impact).
  • No JSON test fixtureTestSSOFlowDefinitionBuilds now constructs the flow definition inline as a Go literal (matching the dominant pattern in graph_builder_test.go), and testdata/sso_flow.json is removed. The test still runs against the real executor registry, so a typo'd executor name or dangling node reference still fails it.
  • Reuse the fetched flow version — the active flow version is now captured from the flow already fetched at context load (initContext / loadContextFromStore) instead of re-fetching it in a dedicated resolveActiveFlowVersion call. That method and the now-redundant flowUsesSSOSession gate are removed; the version is carried on EngineContext.SSOFlowVersion the same way the inbound handle already is.

Related Issues

Related PRs

Checklist

  • Followed the contribution guidelines.
  • Manual test round performed and verified.
  • Documentation provided. (Add links if there are any)
    • Ran Vale and fixed all errors and warnings
  • Tests provided. (Add links if there are any)
    • Unit Tests
    • Integration Tests
  • Breaking changes. (Fill if applicable)
    • Breaking changes section filled.
    • breaking change label added.

Security checks

  • Followed secure coding standards in WSO2 Secure Coding Guidelines
  • Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.

Summary by CodeRabbit

  • Bug Fixes

    • Improved SSO session compatibility checks by consistently associating sessions with the active flow version.
    • Improved handling of inbound SSO sessions during flow execution.
  • New Features

    • Session context and participant information can now be saved, retrieved, listed, and removed per session checkpoint.
    • Added safeguards against storing session contexts that exceed the supported size limit.

- Merge participant and session-context stores into store.go (single store impl)
- Unify SQL query-constant IDs under one SSO-SESS-NN prefix
- Build the graphbuilder SSO test flow inline; drop testdata/sso_flow.json
- Reuse the flow version captured at context load instead of re-fetching the
  flow in resolveActiveFlowVersion

Refs thunder-id#3779
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Flow execution now captures SSO flow versions during context initialization and applies inbound SSO handles directly. Session contexts and participants are persisted through operation-DB store methods, query identifiers are renumbered, and related tests use inline flow definitions.

Changes

SSO execution and session persistence

Layer / File(s) Summary
Initialize SSO execution context
backend/internal/flow/flowexec/model.go, backend/internal/flow/flowexec/service.go
SSO flow versions are initialized from flow.ActiveVersion, inbound handles are applied to the engine context, and runtime graph scanning/version resolution is removed.
Persist session contexts and participants
backend/internal/flow/session/store.go, backend/internal/flow/session/participant_store.go, backend/internal/flow/session/session_context_store.go
Session context and participant creation, retrieval, listing, deletion, serialization, and row mapping are implemented in the operation-DB-backed store.
Align session query identifiers
backend/internal/flow/session/store_constants.go
Session, session-context, and participant query IDs are reassigned without changing their SQL.
Update execution and graph test fixtures
backend/internal/flow/flowexec/service_sso_test.go, backend/internal/flow/graphbuilder/graph_builder_test.go, backend/internal/flow/graphbuilder/testdata/sso_flow.json
Removed obsolete SSO resolution tests and replaced the JSON flow fixture with an inline Go definition.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: ThaminduDilshan, brionmario, DonOmalVindula, Malith-19

Sequence Diagram(s)

sequenceDiagram
  participant RequestContext
  participant flowExecService
  participant EngineContext
  participant FlowDefinition
  RequestContext->>flowExecService: provide inbound SSO handle
  flowExecService->>EngineContext: applyInboundSSO
  FlowDefinition->>EngineContext: provide ActiveVersion
  flowExecService->>EngineContext: initialize or load SSOFlowVersion
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and generic, so it does not clearly communicate the main change. Rename it to reflect the core change, such as merging session stores or simplifying browser SSO flow execution.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description matches the required template and includes the key sections, with only optional checklist items left unchecked.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.72131% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
backend/internal/flow/session/store.go 96.66% 2 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@madurangasiriwardena madurangasiriwardena changed the title Address follow-up review comments on browser SSO Improvements on browser SSO Jul 10, 2026
@madurangasiriwardena
madurangasiriwardena added this pull request to the merge queue Jul 10, 2026
Merged via the queue into thunder-id:main with commit 6e45290 Jul 10, 2026
24 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants