Skip to content

feat(SCIM): user endpoints#12530

Merged
Dschoordsch merged 8 commits intomasterfrom
feat/11051/SCIMUserEndpoints
Feb 5, 2026
Merged

feat(SCIM): user endpoints#12530
Dschoordsch merged 8 commits intomasterfrom
feat/11051/SCIMUserEndpoints

Conversation

@Dschoordsch
Copy link
Copy Markdown
Contributor

@Dschoordsch Dschoordsch commented Feb 2, 2026

Description

Fixes #11051

Demo

[If possible, please include a screenshot or gif/video, it'll make it easier for reviewers to understand the scope of the changes and how the change is supposed to work. If you're introducing something new or changing the existing patterns, please share a Loom and explain what decisions you've made and under what circumstances]

Testing scenarios

[Please list all the testing scenarios a reviewer has to check before approving the PR]

  • Scenario A

    • Step 1
    • Step 2...
  • Scenario B

    • Step 1
    • Step 2....

Final checklist

  • I checked the code review guidelines
  • I have added Metrics Representative as reviewer(s) if my PR invovles metrics/data/analytics related changes
  • I have performed a self-review of my code, the same way I'd do it for any other team member
  • I have tested all cases I listed in the testing scenarios and I haven't found any issues or regressions
  • Whenever I took a non-obvious choice I added a comment explaining why I did it this way
  • I added the label Skip Maintainer Review Indicating the PR only requires reviewer review and can be merged right after it's approved if the PR introduces only minor changes, does not contain any architectural changes or does not introduce any new patterns and I think one review is sufficient'
  • PR title is human readable and could be used in changelog

@Dschoordsch Dschoordsch changed the title Feat/11051/SCIM user endpoints feat(SCIM): user endpoints Feb 2, 2026
@Dschoordsch Dschoordsch changed the base branch from master to feat/11050/SCIMAuthentication February 2, 2026 19:28
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was taken from Okta (referenced here). I removed 1 optional group test and fixed one which passed an invalid email but was supposed to fail on something else.
It's a Runscope/Radar spec, but the test parses this file and runs it.

Comment thread packages/server/scim/SCIMHandler.ts Outdated
Comment thread packages/server/scim/SCIMHandler.ts Outdated
Comment thread packages/server/scim/UserDegress.ts Outdated
Comment thread packages/server/scim/UserDegress.ts Outdated
Comment on lines +24 to +26
if (!user || user.isRemoved) {
throw new SCIMMY.Types.Error(404, '', 'User not found')
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

+1 we need active for Okta, so maybe we should do soft delete to please MS folks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

still valid comment? did you mean Entra by chance? or do both okta and MS require it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. Okta requires it to be present for egress. It actually doesn't properly delete users, so we'll have to implement this in a follow-up.
For Entra it's configurable but recommended to allow deactivation of users.

Comment thread packages/server/scim/UserDegress.ts Outdated
Comment thread packages/server/scim/UserEgress.ts Outdated
Comment thread packages/server/scim/UserIngress.ts Outdated
Comment thread packages/server/scim/UserIngress.ts Outdated
@github-actions github-actions Bot added size/xl and removed size/xl labels Feb 3, 2026
@Dschoordsch Dschoordsch marked this pull request as ready for review February 3, 2026 09:21
Comment thread packages/server/scim/README.md
Comment thread packages/server/scim/SCIMHandler.ts
Comment thread packages/server/scim/SCIMHandler.ts Outdated
req.getHeader('x-application-authorization') || req.getHeader('authorization')
const token = authHeader?.slice(7)

// We're only peaking into the token here to check that it's for SCIM and so we can fetch the SAML config to verify it
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 what's the harm in calling getReqAuth up here? if it's an empty token, then we know it's a 401 & we can remove the check down on L88

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You can chose OAuth or a simple bearer token for SCIM. If the user choses bearer token, then I want only 1 token to be valid at any time, the one stored in the DB. I want to make it a JWT so I can encode the scimId in it so all tenants can use the same endpoint. I don't want to sign it so it's still valid after a server secret rotation. That's why I only peak in it without checking the signature here and only check it for OAuth, for bearer token we check with the DB.

Comment thread packages/server/scim/SCIMHandler.ts Outdated
Comment thread packages/server/scim/UserEgress.ts Outdated
Comment thread packages/server/scim/UserEgress.ts Outdated
Comment thread packages/server/scim/UserEgress.ts
Comment thread packages/server/scim/UserIngress.ts Outdated
Comment thread packages/server/yoga.ts
Base automatically changed from feat/11050/SCIMAuthentication to master February 4, 2026 08:25
@Dschoordsch Dschoordsch force-pushed the feat/11051/SCIMUserEndpoints branch from 5633613 to bb94b19 Compare February 4, 2026 08:28
@github-actions github-actions Bot added size/xl and removed size/xl labels Feb 4, 2026
@github-actions github-actions Bot added size/xl and removed size/xl labels Feb 4, 2026
@Dschoordsch Dschoordsch requested a review from mattkrick February 4, 2026 15:32
Comment on lines +24 to +26
if (!user || user.isRemoved) {
throw new SCIMMY.Types.Error(404, '', 'User not found')
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

still valid comment? did you mean Entra by chance? or do both okta and MS require it?

@Dschoordsch Dschoordsch merged commit 153dab4 into master Feb 5, 2026
8 checks passed
@Dschoordsch Dschoordsch deleted the feat/11051/SCIMUserEndpoints branch February 5, 2026 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SCIM: User GET, PUT, DELETE

2 participants