Skip to content

fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail when synced through SCIM#18384

Merged
keithwillcode merged 7 commits intomainfrom
okta-sync-fixes
Jan 29, 2025
Merged

fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail when synced through SCIM#18384
keithwillcode merged 7 commits intomainfrom
okta-sync-fixes

Conversation

@hariombalhara
Copy link
Copy Markdown
Member

@hariombalhara hariombalhara commented Dec 27, 2024 •
edited
Loading

What does this PR do?

Ensure SCIM sync doesn't suffix domain name if the email is of orgAutoAcceptEmail domain
i.e. john@acme.com -> should get john username instead of john-acme. john@example.com when invited to Acme, would still get john-example username

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Setup Okta sync and create a user from there.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Dec 27, 2024 •
edited
Loading

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "Okta sync fixes". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Dec 27, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 11, 2025

This PR is being marked as stale due to inactivity.

@github-actions github-actions Bot added Stale and removed Stale labels Jan 11, 2025
@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 25, 2025 •
edited
Loading

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Jan 25, 2025 8:41am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Jan 25, 2025 8:41am

hariombalhara
hariombalhara commented Jan 25, 2025
View reviewed changes
Comment thread packages/features/auth/lib/next-auth-options.ts

const usernameSlug = (username: string) => `${slugify(username)}-${randomString(6).toLowerCase()}`;
const getDomainFromEmail = (email: string): string => email.split("@")[1];
const getVerifiedOrganizationByAutoAcceptEmailDomain = async (domain: string) => {
Copy link
Copy Markdown
Member Author

@hariombalhara hariombalhara Jan 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Moved to OrganizationRepository as it is reused

@hariombalhara hariombalhara force-pushed the okta-sync-fixes branch 2 times, most recently from 0330fff to 16d149e Compare January 25, 2025 07:59
hariombalhara
hariombalhara commented Jan 25, 2025
View reviewed changes
Comment thread packages/features/auth/lib/next-auth-options.ts
if (org) {
const createUsersAndConnectToOrgProps = {
emailsToCreate: [email],
organizationId,
Copy link
Copy Markdown
Member Author

@hariombalhara hariombalhara Jan 25, 2025

Choose a reason for hiding this comment

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

It is present in org now.

@hariombalhara hariombalhara changed the title Okta sync fixes fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail Jan 25, 2025
@hariombalhara hariombalhara marked this pull request as ready for review January 25, 2025 08:46
@graphite-app graphite-app Bot requested a review from a team January 25, 2025 08:46
@dosubot dosubot Bot added authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working labels Jan 25, 2025
@hariombalhara hariombalhara changed the title fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail when synced through SCIM Jan 25, 2025
@graphite-app
Copy link
Copy Markdown

graphite-app Bot commented Jan 25, 2025 •
edited
Loading

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (01/25/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (01/29/25)

1 label was added to this PR based on Keith Williams's automation.

alishaz-polymath
alishaz-polymath approved these changes Jan 29, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 29, 2025 •
edited
Loading

E2E results are ready!

@keithwillcode keithwillcode merged commit ac8b4a2 into main Jan 29, 2025
@keithwillcode keithwillcode deleted the okta-sync-fixes branch January 29, 2025 15:05
keithwillcode pushed a commit that referenced this pull request Jan 29, 2025
@hariombalhara @keithwillcode
fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail when…
8b1849f 
… synced through SCIM (#18384)

* Okta fixes

* Fix username and name in case of sync from Okta

* Fix username and name in case of sync from Okta

* Add test

* cleanup code and add tests
MuhammadAimanSulaiman pushed a commit to hit-pay/cal.com that referenced this pull request Feb 25, 2025
@hariombalhara @MuhammadAimanSulaiman
fix: Wrong username with -{DOMAIN} in case of an autoAcceptEmail when…
08e5ecf 
… synced through SCIM (calcom#18384)

* Okta fixes

* Fix username and name in case of sync from Okta

* Fix username and name in case of sync from Okta

* Add test

* cleanup code and add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alishaz-polymath alishaz-polymath alishaz-polymath approved these changes

@Udit-takkar Udit-takkar Awaiting requested review from Udit-takkar

Assignees

No one assigned

Labels

authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in 🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO ready-for-e2e

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hariombalhara @alishaz-polymath @keithwillcode