Skip to content

fix: SCIM reconciliation fixes and diagnostic logging #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TerrifiedBug merged 1 commit into from
Mar 8, 2026
Merged

fix: SCIM reconciliation fixes and diagnostic logging #65

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ async function getAuthInstance() {
userGroupNames = tokenGroups;
}

console.log(`[oidc] User ${user.email} scimEnabled=${settings.scimEnabled}, final groups:`, userGroupNames);
const { reconcileUserTeamMemberships } = await import("@/server/services/group-mappings");
await prisma.$transaction(async (tx) => {
await reconcileUserTeamMemberships(tx, dbUser.id, userGroupNames);
Expand Down
4 changes: 4 additions & 0 deletions src/server/services/group-mappings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export async function reconcileUserTeamMemberships(
userGroupNames: string[],
): Promise<void> {
const allMappings = await loadGroupMappings();
console.log(`[reconcile] userId=${userId}, userGroups=${JSON.stringify(userGroupNames)}, mappings=${JSON.stringify(allMappings)}`);
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 8, 2026

Choose a reason for hiding this comment

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

Full system-wide RBAC config dumped on every reconcile call

allMappings contains every group-to-team mapping in the system — not just the ones relevant to this user. This function is called on every SSO login and on every SCIM group membership change, so in a busy deployment you'll get a full dump of your entire RBAC configuration in the logs on each event.

Two considerations:

  1. Verbosity: in a deployment with many SCIM users this will generate very noisy logs and make the entries you actually care about harder to find.
  2. Scope of exposure: server logs that contain the full mapping table (all teamIds, group names, and assigned roles) represent a broader blast radius if log storage is ever misconfigured or accessed by the wrong party, compared to logging only the subset of mappings that matched the current user's groups.

Consider scoping the log to the mappings that actually matched this user:

const matchedMappings = allMappings.filter(m => userGroupNames.includes(m.group));
console.log(`[reconcile] userId=${userId}, userGroups=${JSON.stringify(userGroupNames)}, matchedMappings=${JSON.stringify(matchedMappings)}`);

This still reveals which mappings fired and which groups matched, giving you all the diagnostic signal you need without dumping the full config table every time.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: src/server/services/group-mappings.ts
Line: 59

Comment:
**Full system-wide RBAC config dumped on every reconcile call**

`allMappings` contains every group-to-team mapping in the system — not just the ones relevant to this user. This function is called on every SSO login *and* on every SCIM group membership change, so in a busy deployment you'll get a full dump of your entire RBAC configuration in the logs on each event.

Two considerations:
1. **Verbosity**: in a deployment with many SCIM users this will generate very noisy logs and make the entries you actually care about harder to find.
2. **Scope of exposure**: server logs that contain the full mapping table (all teamIds, group names, and assigned roles) represent a broader blast radius if log storage is ever misconfigured or accessed by the wrong party, compared to logging only the subset of mappings that matched the current user's groups.

Consider scoping the log to the mappings that actually matched this user:
```typescript
const matchedMappings = allMappings.filter(m => userGroupNames.includes(m.group));
console.log(`[reconcile] userId=${userId}, userGroups=${JSON.stringify(userGroupNames)}, matchedMappings=${JSON.stringify(matchedMappings)}`);
```
This still reveals which mappings fired and which groups matched, giving you all the diagnostic signal you need without dumping the full config table every time.

How can I resolve this? If you propose a fix, please make it concise.


// Compute desired state: for each team, the highest role from any matching group
const desiredTeamRoles = new Map<string, "VIEWER" | "EDITOR" | "ADMIN">();
Expand All @@ -69,10 +70,13 @@ export async function reconcileUserTeamMemberships(
}
}

console.log(`[reconcile] desiredTeamRoles=${JSON.stringify([...desiredTeamRoles.entries()])}`);

// Fetch current group_mapping TeamMembers for this user
const existing = await tx.teamMember.findMany({
where: { userId, source: "group_mapping" },
});
console.log(`[reconcile] existing group_mapping members=${JSON.stringify(existing.map(m => ({ teamId: m.teamId, role: m.role })))}`);

const existingByTeam = new Map(existing.map((m) => [m.teamId, m]));

Expand Down
Loading