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

Allow orgs to define role when creating and updating groups via SCIM #1909

Merged
merged 7 commits into from Dec 11, 2023

Conversation

mknowlton89
Copy link
Collaborator

@mknowlton89 mknowlton89 commented Dec 1, 2023

Features and Changes

It's been requested now that we've added the ability for organizations using SCIM to define a growthbookRole when creating/updating users, that we do the same thing for Teams.

To do this, we've added a new optional property to the /scim/v2/groups POST and PATCH requests. This property, like the user property we just added is growthbookRole.

It's important to note that currently, Okta is the only Identity Provider that GrowthBook has SCIM support for officially, and, Okta doesn't offer the ability to include custom attributes when creating/updating groups. Which means that currently, no organizations using Okta's SCIM application can take advantage of this.

However, its possible other organizations are using SCIM with Identity Providers other than Okta that do support this, so we want to extend support. It'll be helpful for a few organizations now, and also make our SCIM integration more flexible as time goes on.

To take advantage of this new functionality, here are two sample API requests that have been tested.

Create Group

Request Type: POST
Endpoint: https://SCIM_BASE_URL/scim/v2/groups
Request Body:

{
    "schemas": [
        "urn:ietf:params:scim:schemas:core:2.0:Group"
    ],
    "displayName": "Test Team",
    "growthbookRole": "readonly",
    "members": [
        {
            "value": "userid_123",
            "display": "email@address.com"
        }
    ]
}

Update Group

Request Type: PATCH
Endpoint: https://SCIM_BASE_URL/scim/v2/groups/:teamId
Request Body:

{
    "schemas": [
        "urn:ietf:params:scim:schemas:core:2.0:PatchOp"
    ],
    "Operations": [
        {
            "op": "replace",
            "value": {
                "id": "teamId",
                "displayName": "Test Team Name",
                "growthbookRole": "admin"
            }
        }
    ]
}

Testing

  • Create a new team via Postman with the request above, minus the growthbookRole property and ensure that it's created successfully and the team's global role is the organizations default role.
  • Now do the same thing, but instead include the growthbookRole property set to "admin" and ensure the team is created successfully and the team's global role is admin.
  • Now do the same thing, but give it an invalid growthbookRole and ensure that an error is thrown.
  • [x]Now, update an existing team with a valid role, and ensure that the role is updated correctly
  • Now, update an existing team, but this time, only update the team name and ensure it works correctly
  • Now, update an existing team with a bogus role and ensure an error is thrown
  • Within a working Okta SCIM application, ensure you can still provision/de-provision users without issue, and can still push and un-sync groups without issue.

Copy link

github-actions bot commented Dec 1, 2023

Your preview environment pr-1909-bttf has been deployed.

Preview environment endpoints are available at:

Copy link

github-actions bot commented Dec 6, 2023

Deploy preview for docs ready!

✅ Preview
https://docs-4mu5ronzg-growthbook.vercel.app

Built with commit 0c81b74.
This pull request is being automatically deployed with vercel-action

@mknowlton89 mknowlton89 changed the title Scaffolds out supporting setting a role when creating a group via the… Allow orgs to define role when creating and updating groups via SCIM Dec 6, 2023
@mknowlton89 mknowlton89 self-assigned this Dec 6, 2023
@mknowlton89 mknowlton89 marked this pull request as ready for review December 6, 2023 21:50
@mknowlton89
Copy link
Collaborator Author

mknowlton89 commented Dec 8, 2023

Quick note: I made a change in this PR that moves the isValidRole check to the shared folder. I also broke the function out into two methods in order to support the new SCIM role that the PR linked will introduce. Depending on if that PR lands before this, there'll be conflicts that I will need to resolve, but it'll just be changing the import location.

Copy link
Contributor

@msamper msamper left a comment

Choose a reason for hiding this comment

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

Just a minor comment about simplifying the update logic, but otherwise lgtm!

updatedTeam.role = growthbookRole;
}

await updateTeamMetadata(team.id, org.id, updatedTeam);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can replace this with

        await updateTeamMetadata(team.id, org.id, {
          ...team,
          name: (value as BasicScimGroup).displayName,
          managedByIdp: true,
          growthbookRole,
        });

and have the same effect without needing cloneDeep. If its undefined it just wont be added and if its not a valid role we'll return the error before we get to update

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, yea - much cleaner. Thanks!

@mknowlton89 mknowlton89 merged commit 62d9490 into main Dec 11, 2023
6 checks passed
@mknowlton89 mknowlton89 deleted the mk/specify-role-for-teams branch December 11, 2023 13:53
bryce-fitzsimons pushed a commit that referenced this pull request Dec 12, 2023
…1909)

* Adds support for setting team/group global role when creating a team or updating a team via SCIM endpoints
bryce-fitzsimons added a commit that referenced this pull request Jan 12, 2024
* initial, updating existing types and methods with new bucket stuff

* moar types

* simplify targeting ui, remove stickyBucketing var

* add minBucketVersion, add reseed option for new phases

* org setting for sticky bucketing, premium feature, organize settings page

* lock SB switch based if !hasCommercialFeature

* working on rollout recommendation logic

* variation blocking ui, wip

* tweak logic kinda

* tweak

* re-add disableStickyBucketing

* ui wip

* ui wip

* ui wip

* ui wip

* ui wip

* ui wip

* ui wip

* ui wip

* Support ability to set role with SCIM request. (#1779)

* Support ability to set role with SCIM request.

* Modify cascading window logic for excluding in-progress conversions (#1921)

* Modify cascading window logic for excluding in-progress conversions

* Move fn

* Test

* Fix unnecessary activation fact metric data retrieval (and two minor FE bugs) (#1922)

* Update conversion window logic for fact metrics

* CLean up language

* More proximate fix

* Pre-compute safe dimension slices (#1873)

* Add reliable dimensions query

* Modal mostly working

* Get save working

* lint and such

* merge origin main

* Wire auto dimensions through to units and traffic query

* remove dimensions for traffic

* Fix SQL

* Bryce comments

* Move to kebab & add context

* rename && remove ff

* remove FF

* Table version

* ui/code tweaks

* Add running language

* remove console

* Remove metadata field

* Additional renaming

* Hide entry with no dimensions

* Move storing specified values directly to exposure query

* TS

* Small updates

* Fix type issue

* Rename && add lookback selector

* Cleanup

* push pixels

* Add tracking

* Blank out button for automatic dimensions

* Ensure cast to string before string comparison

---------

Co-authored-by: Bryce <bryce1@gmail.com>

* Add new rollback workflow (#1867)

* Fix bug - metric capping cannot be updated on old metrics (#1930)

* Add Random On-Screen Celebration Service (#1826)

* Add on-screen confetti celebration service to app. Launch experiment consumes it and will fire 20% of the time.

* Allow orgs to define role when creating and updating groups via SCIM (#1909)

* Adds support for setting team/group global role when creating a team or updating a team via SCIM endpoints

* SDK compatibility versioning (#1893)

* initial: new sdkVersion field, sdkid/form UI

* sdk-versioning package, getCurrentVersion method

* getCapabilities

* payload scrubbing. mostly feature complete

* BE resolveJsonModule

* update scrubbing logic to use pick

* refactor json

* isOutdated check

* add all sdks, fill in versions/capabilities

* tweaks

* get minimal safe capabilities when multi-language, add tests

* basic gating in SDK connection form

* more retrofitting

* more retrofitting

* address feedback

* address feedback

* lint

* use today's capabilities as a snapshot for default sdk version

* small fixes

* clean up scrubbing, add tests

* fix sdkconnection post endpoint

* fix model option version

* api types

* fix test

* multi-stage scrubbing, update legacy connection logic, tests

* update php for looseUnmarshalling

* address feedback

* log payload deltas instead of scrubbing until confident

* use logger

* log capabilities

* use isEqual for better payload scrub comparison (#1935)

* ui wip

* resize release plan summary. probably redo this later...

* fix hook deps

* ui wip

* block save when no changes, cleanup old code

* more clarity around SB SDK support

* refactor models based on discussions (no blockedVariations, new bool)

* wip multi-stage modal

* wip multi-stage modal

* wip multi-stage modal

* wip multi-stage modal

* wip multi-stage modal

* wip multi-stage modal

* wip multi-stage modal

* wip multi-stage modal

* confirm check

* add warnings while making changes

* cleanup

* cleanup

* fine tune ui

* remove blocked from variation (add later)

* clean up modal ui

* payload scrubbing

* remove reseed from phases modals, fix any type

* attempt saved group change detection

* lint

* move warning to release plan page, don't warn when using recommended, ui tweaks

* change targeting language, ui tweaks

* fix saved group comparison, fix release plan selector, adjust ui

* wip SB onboarding / toggle

* sticky bucketing onboarding and toggle gating + warnings

* phases: looking for targeting? -> make changes

* refactor

* retool warnings

* address some feedback

* adjust settings SB toggle

* fix saved group restrictive ANY calc

* fix saved group restrictive ANY calc again

* make changes to choices

* other fix

* tweaks

* calculate risk level per option

* phase and warning stuff

* ui tweak, edit phases show/hide button logic

* adjust warning language

* move same-phase-everyone into default release plan options

* add SB keys to payload scrubber

* implement some suggestions

* fix legacy webhook capabilities

* fix help text

* fix help text again

* update types

* ui tweaks

* remove onboarding CTAs when sdk issues detected, point to docs

* gate fallback attributes by org setting

* UI tweaks for sticky bucket onboarding and Make Changes modal

* minor ui tweaks

* Sticky bucketing docs (#2017)

* talk about new stuff in experiment-configuration, fix proxy docs (unrelated)

* sb doc stub

* sb doc stub

* sb doc wip

* change slug

* sb doc wip

* sb doc wip

* sb doc wip

* sdk docs

* Language

* de-emphasize fallbackAttribute in examples

* Update sticky bucket docs page with fallback attribute example and helper image for SDK connections

---------

Co-authored-by: Luke Sonnet <luke.sonnet@gmail.com>
Co-authored-by: Jeremy Dorn <jeremy@jeremydorn.com>

---------

Co-authored-by: Michael Knowlton <michael@growthbook.io>
Co-authored-by: Luke Sonnet <lukesonnet@users.noreply.github.com>
Co-authored-by: Adnan Chowdhury <2374625+bttf@users.noreply.github.com>
Co-authored-by: Jeremy Dorn <jeremy@jeremydorn.com>
Co-authored-by: Luke Sonnet <luke.sonnet@gmail.com>
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.

None yet

2 participants