Skip to content

[FEATURE] FWF-6078 - Multi-Tenant Single Login#1147

Draft
salabh-aot wants to merge 1 commit intodevelopfrom
feature/fwf-6078
Draft

[FEATURE] FWF-6078 - Multi-Tenant Single Login#1147
salabh-aot wants to merge 1 commit intodevelopfrom
feature/fwf-6078

Conversation

@salabh-aot
Copy link

@salabh-aot salabh-aot commented Mar 25, 2026

User description

📝 Pull Request Summary

Description:

Unified login experience for multitenant environment

Related Jira Ticket:


Type of Change:

  • ✨ Feature
  • 🐞 Bug Fix
  • ♻️ Refactor
  • 🧹 Code Cleanup
  • 🧪 Test Addition / Update
  • 📦 Dependency Update
  • 📘 Documentation Update
  • 🚀 Deployment / Config / Security Updates

🧩 Microfrontends Affected

Please select all microfrontends or modules that are part of this change:

  • forms-flow-admin
  • forms-flow-components
  • forms-flow-integration
  • forms-flow-nav
  • forms-flow-review
  • forms-flow-service
  • forms-flow-submissions
  • forms-flow-theme
  • scripts
  • Others (please specify below)

Details (if Others selected):


🧠 Summary of Changes

Updated keycloak service to always use default client and not use ${tenantId}-<default-client>

🧪 Testing Details

Testing Performed:

Screenshots (if applicable):


✅ Checklist

  • Code builds successfully without lint or type errors
  • Unit tests added or updated
  • Integration or end-to-end tests validated
  • UI verified
  • Documentation updated (README / Confluence / UI Docs)
  • No sensitive information (keys, passwords, secrets) committed
  • I have updated the CHANGELOG with relevant details
  • I have given a clear and meaningful PR title and description
  • I have verified cross-repo dependencies (if any)
  • I have confirmed backward compatibility with previous releases
  • Verified changes across all selected microfrontends

👥 Reviewer Notes


PR Type

Bug fix, Enhancement


Description

  • Use shared Keycloak client across tenants

  • Remove tenantId from service creation

  • Log token parse failures before logout


Diagram Walkthrough

flowchart LR
  app["App startup"]
  instance["KeycloakService.getInstance"]
  config["Default clientId configuration"]
  auth["Keycloak authentication"]
  log["Token parse failure logging"]
  app -- "requests service" --> instance
  instance -- "creates with" --> config
  config -- "used by" --> auth
  auth -- "on parse failure" --> log
Loading

File Walkthrough

Relevant files
Enhancement
KeycloakService.ts
Simplify Keycloak client selection for tenants                     

forms-flow-service/src/keycloak/KeycloakService.ts

  • Removed tenantId from the constructor signature
  • Removed tenantId from getInstance parameters
  • Always sets Keycloak clientId to the default value
  • Added a console log before logout on token parse failure
+4/-4     

@salabh-aot salabh-aot added the Do not merge X This PR shouldnt be merged label Mar 25, 2026
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Preserve tenant-specific client IDs

This change drops tenant-aware client resolution, so every login will use the same
Keycloak client regardless of tenant. In a multi-tenant flow, that can send users to
the wrong client configuration and break authentication or token validation.

forms-flow-service/src/keycloak/KeycloakService.ts [20-29]

 private constructor(
   url: string,
   realm: string,
   clientId: string,
+  tenantId?: string
 ) {
   this._keycloakConfig = {
     url: url,
     realm: realm,
-    clientId: clientId, // tenantId ? `${tenantId}-${clientId}` : clientId,
+    clientId: tenantId ? `${tenantId}-${clientId}` : clientId,
   };
Suggestion importance[1-10]: 8

__

Why: The PR removes tenantId from the constructor and hardcodes clientId, which can break existing multi-tenant Keycloak client resolution. The suggestion is consistent with the removed logic and addresses a likely authentication regression.

Medium
Recreate service for config changes

Returning a single cached instance without considering tenant or client changes
means the first initialized tenant/client can be reused for later requests. That can
cause cross-tenant authentication, refresh, and logout calls to run against the
wrong Keycloak configuration.

forms-flow-service/src/keycloak/KeycloakService.ts [123-131]

 public static getInstance(
   url: string,
   realm: string,
   clientId: string,
+  tenantId?: string
 ): KeycloakService {
-  return this.instance
-    ? this.instance
-    : (this.instance = new KeycloakService(url, realm, clientId));
+  const resolvedClientId = tenantId ? `${tenantId}-${clientId}` : clientId;
+
+  if (
+    !this.instance ||
+    this.instance._keycloakConfig.url !== url ||
+    this.instance._keycloakConfig.realm !== realm ||
+    this.instance._keycloakConfig.clientId !== resolvedClientId
+  ) {
+    this.instance = new KeycloakService(url, realm, clientId, tenantId);
+  }
+
+  return this.instance;
 }
Suggestion importance[1-10]: 8

__

Why: Caching a single KeycloakService instance regardless of url, realm, or clientId can reuse the wrong configuration on later calls. The suggestion directly fixes a real correctness issue in getInstance, especially for multi-client or multi-tenant flows.

Medium

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

Labels

Do not merge X This PR shouldnt be merged Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant