Skip to content
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.

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

feat: SCIM 2.0 Automatic User provisioning #11169

Merged
merged 71 commits into from
Mar 6, 2024
Merged

Conversation

devkiran
Copy link
Contributor

@devkiran devkiran commented Sep 6, 2023

What does this PR do?

Fixes #83

Requirement/Documentation

  • If there is a requirement document, please, share it here.
  • If there is ab UI/UX design document, please, share it here.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How should this be tested?

  • Ensure that NEXT_PUBLIC_HOSTED_CAL_FEATURES=1 and SAML_DATABASE_URL is set
  • Create an org
  • Go to "http://app.cal.local:3000/settings/organizations/dsync" and create a dsync connection for Otka
  • In Bitwarden we have a dev Okta account, log in to Otka at the URL associated with the credentials
  • From the sidebar go to Applications -> Applications
  • Select Calcom Local
  • From the menu go to Provisioning -> Integration
  • Hit Edit
    • From your machine use Ngrok to create a tunnel to your localhost
    • On the org dsync data enter the URL under "SCIM connector base URL" but change the base URL to the ngroked one
    • Under "Authorization" enter the "SCIM Bearer Token" from the org dsync data
  • Go to assignments and assign a new user to the app
  • An org invitation & signup email should have been sent out if the user is not a part of the org
  • If the user already exists then an invite email should have been sent

Mandatory Tasks

  • Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

@vercel
Copy link

vercel bot commented Sep 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2023 5:09am

@vercel
Copy link

vercel bot commented Sep 6, 2023

@devkiran is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

📦 Next.js Bundle Analysis for @calcom/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

New Page Added

The following page was added to the bundle from the code in this PR:

Page Size (compressed) First Load % of Budget (350 KB)
/settings/organizations/dsync 266.69 KB 455 KB 130.00%

Fourteen Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load % of Budget (350 KB)
/500 90.54 KB 278.86 KB 79.67% (🟡 +0.16%)
/auth/forgot-password 102.51 KB 290.82 KB 83.09% (🟡 +0.15%)
/auth/forgot-password/[id] 106.84 KB 295.16 KB 84.33% (🟡 +0.15%)
/auth/verify-email-change 90.12 KB 278.44 KB 79.55% (🟡 +0.16%)
/bookings/[status] 325.83 KB 514.15 KB 146.90% (🟢 -0.23%)
/payment/[uid] 119.38 KB 307.69 KB 87.91% (🟡 +0.16%)
/settings/admin/orgMigrations/_OrgMigrationLayout 253.15 KB 441.47 KB 126.13% (🟢 -1.68%)
/settings/my-account/profile 406.83 KB 595.14 KB 170.04% (🟢 -0.20%)
/settings/organizations/profile 398.32 KB 586.63 KB 167.61% (🟢 -0.20%)
/settings/teams/[id]/members 383.32 KB 571.63 KB 163.32% (🟢 -0.23%)
/settings/teams/[id]/profile 470.92 KB 659.23 KB 188.35% (🟢 -0.14%)
/settings/teams/new 199.17 KB 387.48 KB 110.71% (🟡 +0.17%)
/signup 148.63 KB 336.95 KB 96.27% (🟡 +0.17%)
/video/[uid] 235.1 KB 423.41 KB 120.97% (🟡 +0.16%)
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored.

@PeerRich
Copy link
Member

PeerRich commented Sep 7, 2023

whats missing to leave draft?

@devkiran
Copy link
Contributor Author

devkiran commented Sep 8, 2023

@PeerRich Currently, the app receives the users/group events from the identity provider; the next step is to update the database based on the directory sync events. For example, on the user.created event - create a user within the team. We need to handle eight such events as part of the SCIM integration. Someone experienced with users, roles and team management within the Cal can finish the integration. Most of the SCIM-specific works have been finished from our side.

@devkiran
Copy link
Contributor Author

@leog I would Appreciate it if you could take a look at this PR, no rush. Let me know if you have any questions. Thanks!

@leog
Copy link
Contributor

leog commented Sep 27, 2023

Hey @devkiran. Great job so far. Found some things in the front-end, let me know if I should help you with any of these.
Also, in terms of back-end, adding some notes so we can ensure the path to take for this integration.

Front-end issues (cc our Head of Product @ciaranha):

  1. [Medium] Error toast with env variables missing, maybe this should be kind of an empty screen of this section to show the blocker.

  2. [Minor] Missing i18n

  3. [Minor] Cutting field border

  4. [Medium] Error toast about not having permission, also should be kind of an empty screen of this section to show the blocker.

Back-end notes

  1. Per your description in the PR, I just wanted to confirm that for self-hosters, we want to sync users to the database without any connection to teams whatsoever.
    • The tenant comes as "Cal.com" which signals users should be created without a linked team
  2. Per your description in the PR, I just wanted to confirm that for Cal.com, we want to sync users to the database connecting them to a specific team.
    • The tenant for a group "Acme" comes as "team-2" in my tests, we can't really infer to which team new users should be linked to on the hosted version.
  3. Given the different events such as updating user attributes, deactivate users, sync password, what do you recommend to do in each event? For example, we don't have the ability to deactivate users, would it be safe to assume a deletion instead?
  4. As we discussed, we need to define the connection between created users and organizations. We could make that connection based on the domain name of the email addresses of the users we get in the events. If there is an organization using "acme.com" email addresses, and an event user.created is triggered, and that user has an "acme.com" email address, then we create that user to be linked to that org. I guess this will also be working on Cal.com, not for self-hosters.

Thanks.

@devkiran
Copy link
Contributor Author

devkiran commented Oct 3, 2023

@leog Thanks for reviewing the PR, I have added the changes you suggested.

Please see my comments below.

  1. You're right about the self-hosted instance. We use the tenant ID as Cal.com; users created without any team.

  2. The format we used here is ${tenantPrefix}${input.teamId}. The tenant prefix is team- and the team ID is 2, the tenant would be team-2. I followed it because this is what we used in SAML integration. We can change it as per your suggestion Refer

  3. Here are the various events Jackson sent based on the changes that happened to the identity provider. You can safely remove the users if they are deactivated; you'll receive a 'users.updated' event with active: false. I can help you understand each events if needed.

  4. Yes, I think that would work. Let me know if you need any help with that.

@leog
Copy link
Contributor

leog commented Oct 3, 2023

@devkiran Thank you for replying. Understood the flows. I will see now who should be tackling the mapping with each event.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2023

Hey there, there is a merge conflict, can you take a look?

@github-actions github-actions bot added the 🚨 merge conflict This PR has a merge conflict that has to be addressed label Oct 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2023

Hey there, there is a merge conflict, can you take a look?

keithwillcode and others added 2 commits March 4, 2024 15:45
* Rename .env variables

* Remove instances of old variable names

* Add credential sync secret to request for new token

* Update invalid secret message

Co-authored-by: Keith Williams <keithwillcode@gmail.com>

---------

Co-authored-by: Keith Williams <keithwillcode@gmail.com>
Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

It looks good to me now 🙏 As it has the high-risk label I am waiting with my approval until someone from @calcom/foundation looks over it first

@CarinaWolli CarinaWolli dismissed their stale review March 5, 2024 00:26

All requested changes are fixed

if (tab.name === "security" && !HOSTED_CAL_FEATURES) {
tab.children?.push({ name: "sso_configuration", href: "/settings/security/sso" });
// TODO: Enable dsync for self hosters
// tab.children?.push({ name: "directory_sync", href: "/settings/security/dsync" });
Copy link
Member

Choose a reason for hiding this comment

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

Do we need these comments? Since we already have it on line 103?


const UNSTABLE_HANDLER_CACHE: DsyncRouterHandlerCache = {};

export const dsyncRouter = router({
Copy link
Member

Choose a reason for hiding this comment

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

Can we use the current desired boilerplate as done here? https://github.com/calcom/cal.com/pull/10849/files

Copy link
Contributor

Choose a reason for hiding this comment

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

Dsync routes shouldn't be public. I noticed that PR only made changes to the public router. I looked at the event type _router file and it's following the same convention.

input,
}: {
usernameOrEmail: string;
team: Awaited<ReturnType<typeof getTeamOrThrow>>;
translation: TFunction;
ctx: { user: NonNullable<TrpcSessionUser> };
inviterName: string;
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

Copy link
Member

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Some code quality comments in there. Still need to test locally.

@zomars zomars merged commit 1064e11 into calcom:main Mar 6, 2024
30 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Created by Linear-GitHub Sync enterprise area: enterprise, audit log, organisation, SAML, SSO ❗️ .env changes contains changes to env variables ✨ feature New feature or request High priority Created by Linear-GitHub Sync high-risk Requires approval by Foundation team ❗️ migrations contains migration files organizations area: organizations, orgs osshack Submission for 2023 OSShack 🚧 wip / in the making This is currently being worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-720] SCIM 2.0 Automatic User provisioning
10 participants