Skip to content

Integrate keycloak#45

Closed
Gomering1 wants to merge 39 commits intodevelopfrom
integrate-keycloak
Closed

Integrate keycloak#45
Gomering1 wants to merge 39 commits intodevelopfrom
integrate-keycloak

Conversation

@Gomering1
Copy link
Collaborator

@Gomering1 Gomering1 commented Feb 25, 2026

Summary by CodeRabbit

  • New Features

    • Keycloak/OIDC sign-in flow (start/callback), logout, and access token session handling
    • Password-based login/registration disabled; Keycloak signup supported with feature flags
    • User presence (online/offline) and users list endpoint
    • Direct messaging and updated home/dashboard flows
    • Backend/frontend Keycloak configuration and environment-driven feature flags
  • Improvements

    • More robust auth-protected API proxying and error handling

Ilia and others added 30 commits February 20, 2026 21:57
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (2)
backend/src/Domain/Users/PendingRegistration.cs (1)

11-15: ⚠️ Potential issue | 🟠 Major

Enforce Email/NormalizedEmail invariant inside the entity.

normalizedEmail being caller-provided in both creation and refresh paths can still desync persisted state. Normalize from email internally to keep the entity consistent and avoid lookup/index mismatches.

🐛 Proposed fix
 public sealed class PendingRegistration
 {
+    private static string NormalizeEmail(string email)
+    {
+        if (string.IsNullOrWhiteSpace(email))
+            throw new ArgumentException("Email is required.", nameof(email));
+        return email.Trim().ToUpperInvariant();
+    }
+
     private PendingRegistration()
     {
     }
 
-    public PendingRegistration(string email, string normalizedEmail)
+    public PendingRegistration(string email)
     {
         Email = email;
-        NormalizedEmail = normalizedEmail;
+        NormalizedEmail = NormalizeEmail(email);
     }
@@
     public void Refresh(
         string email,
-        string normalizedEmail,
         string displayName,
         string passwordHash,
         string verificationCodeHash,
         DateTime codeExpiresAtUtc,
         DateTime codeSentAtUtc)
     {
         Email = email;
-        NormalizedEmail = normalizedEmail;
+        NormalizedEmail = NormalizeEmail(email);
         DisplayName = displayName;
         PasswordHash = passwordHash;
         VerificationCodeHash = verificationCodeHash;

Also applies to: 29-40

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/Domain/Users/PendingRegistration.cs` around lines 11 - 15,
PendingRegistration currently accepts a caller-provided normalizedEmail which
can lead to desyncs; change the entity to derive NormalizedEmail internally from
Email instead of trusting the parameter: update the constructor
PendingRegistration(string email, string normalizedEmail) to take only email (or
ignore the normalizedEmail param) and compute NormalizedEmail via the project’s
canonical email normalization helper (use the same normalization call used
elsewhere), and apply the same change to the refresh/update path referenced
around lines 29-40 so that any Refresh/Update method recomputes NormalizedEmail
from Email; then update callers to stop providing normalizedEmail or ignore it.
backend/docker-compose.yml (1)

13-13: ⚠️ Potential issue | 🟠 Major

Remove the SQL Server default password fallback.

Line 13 still bakes in a known fallback secret (YourStrong@Password). If DatabaseProvider is switched, this becomes an active credential path and weakens security posture.

🔒 Proposed fix
-      - SqlServer__ConnectionString=Server=sqlserver;Database=AzureOpsCrew;User Id=sa;Password=${SQL_SERVER_SA_PASSWORD:-YourStrong@Password};TrustServerCertificate=True;
+      - SqlServer__ConnectionString=${SQLSERVER_CONNECTION_STRING:?SQLSERVER_CONNECTION_STRING must be set}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/docker-compose.yml` at line 13, Remove the baked-in fallback secret
by editing the environment entry that sets SqlServer__ConnectionString: stop
using the inline default `${SQL_SERVER_SA_PASSWORD:-YourStrong@Password}` and
instead reference `${SQL_SERVER_SA_PASSWORD}` (or wire in a Docker secret) so
the hardcoded `YourStrong@Password` is never used; ensure any deployment/startup
validates the variable is present and fails fast if missing.
🧹 Nitpick comments (1)
backend/src/Api/Extensions/ServiceCollectionExtensions.cs (1)

111-113: Duplicate settings deserialization across both extension methods — optional cleanup.

Both AddJwtAuthentication and AddKeycloakOidcSupport independently call configuration.GetSection("KeycloakOidc").Get<KeycloakOidcSettings>(). While benign (both methods are public and may be called independently), if they are always paired there's an opportunity to reduce redundant deserialization.

Also applies to: 174-176

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/Api/Extensions/ServiceCollectionExtensions.cs` around lines 111 -
113, Both AddJwtAuthentication and AddKeycloakOidcSupport duplicate
configuration.GetSection("KeycloakOidc").Get<KeycloakOidcSettings>() — extract
the deserialization into a single shared step and reuse it to avoid redundant
work; for example, create a private static method or overload that returns
KeycloakOidcSettings (used by AddJwtAuthentication and AddKeycloakOidcSupport)
or read it once and pass the KeycloakOidcSettings instance into both methods so
both methods use the same deserialized object instead of calling
configuration.GetSection(...).Get<KeycloakOidcSettings>() independently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/docker-compose.yml`:
- Around line 32-33: The docker-compose service mounts a persistent volume using
the entry "volumes: - ./data:/app/data", so add a blanket ignore for the data
directory to .gitignore (e.g., add a "data/" entry) to prevent any files under
that mount (including SQLite DBs) from being accidentally committed; update the
.gitignore by appending the directory ignore rather than relying only on
individual filenames like azureopscrew.db*.

In `@backend/src/Api/Extensions/ServiceCollectionExtensions.cs`:
- Around line 134-142: Enable audience validation on the JWT options: set
TokenValidationParameters.ValidateAudience = true and configure
TokenValidationParameters.ValidAudience (or ValidAudiences) to the API's client
id so the middleware rejects tokens not intended for this resource; update the
options block that constructs TokenValidationParameters (where ValidateIssuer,
ValidIssuer, ValidateAudience, ValidateLifetime, ClockSkew, NameClaimType are
set) to flip ValidateAudience to true and provide the expected audience
value(s), and ensure Keycloak is configured to emit the aud claim (via an
Audience protocol mapper) before deploying this change.

In `@backend/src/Api/Program.cs`:
- Around line 47-48: The call order in Program.cs makes AddJwtAuthentication run
before AddKeycloakOidcSupport so the settings.Enabled guard inside
AddKeycloakOidcSupport never executes; reorder the service registrations so
AddKeycloakOidcSupport(builder.Configuration, builder.Environment) is invoked
before AddJwtAuthentication(builder.Configuration, builder.Environment),
ensuring the settings.Enabled check in AddKeycloakOidcSupport can run, or
alternatively remove/adjust the redundant Enabled check in
AddKeycloakOidcSupport if you intend AddJwtAuthentication to be authoritative.

---

Duplicate comments:
In `@backend/docker-compose.yml`:
- Line 13: Remove the baked-in fallback secret by editing the environment entry
that sets SqlServer__ConnectionString: stop using the inline default
`${SQL_SERVER_SA_PASSWORD:-YourStrong@Password}` and instead reference
`${SQL_SERVER_SA_PASSWORD}` (or wire in a Docker secret) so the hardcoded
`YourStrong@Password` is never used; ensure any deployment/startup validates the
variable is present and fails fast if missing.

In `@backend/src/Domain/Users/PendingRegistration.cs`:
- Around line 11-15: PendingRegistration currently accepts a caller-provided
normalizedEmail which can lead to desyncs; change the entity to derive
NormalizedEmail internally from Email instead of trusting the parameter: update
the constructor PendingRegistration(string email, string normalizedEmail) to
take only email (or ignore the normalizedEmail param) and compute
NormalizedEmail via the project’s canonical email normalization helper (use the
same normalization call used elsewhere), and apply the same change to the
refresh/update path referenced around lines 29-40 so that any Refresh/Update
method recomputes NormalizedEmail from Email; then update callers to stop
providing normalizedEmail or ignore it.

---

Nitpick comments:
In `@backend/src/Api/Extensions/ServiceCollectionExtensions.cs`:
- Around line 111-113: Both AddJwtAuthentication and AddKeycloakOidcSupport
duplicate configuration.GetSection("KeycloakOidc").Get<KeycloakOidcSettings>() —
extract the deserialization into a single shared step and reuse it to avoid
redundant work; for example, create a private static method or overload that
returns KeycloakOidcSettings (used by AddJwtAuthentication and
AddKeycloakOidcSupport) or read it once and pass the KeycloakOidcSettings
instance into both methods so both methods use the same deserialized object
instead of calling configuration.GetSection(...).Get<KeycloakOidcSettings>()
independently.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ee9c8c and 5d98028.

📒 Files selected for processing (6)
  • backend/docker-compose.yml
  • backend/src/Api/Extensions/ServiceCollectionExtensions.cs
  • backend/src/Api/Program.cs
  • backend/src/Domain/Users/PendingRegistration.cs
  • backend/src/Infrastructure.Db/EntityTypes/Sqlite/UserExternalIdentityEntityTypeConfiguration.cs
  • frontend/app/api/auth/keycloak/callback/route.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • backend/src/Infrastructure.Db/EntityTypes/Sqlite/UserExternalIdentityEntityTypeConfiguration.cs
  • frontend/app/api/auth/keycloak/callback/route.ts

@darkcodi
Copy link
Owner

darkcodi commented Mar 1, 2026

This PR needs rebasing. Also, using KeyCloak will slow down the local development, so let's postpone this change till the end of hackathon.

@darkcodi darkcodi closed this Mar 1, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
Repository owner deleted a comment from coderabbitai bot Mar 7, 2026
@darkcodi darkcodi deleted the integrate-keycloak branch March 15, 2026 22:01
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