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: saml upload #9750

Merged
merged 2 commits into from
May 20, 2024
Merged

feat: saml upload #9750

merged 2 commits into from
May 20, 2024

Conversation

mattkrick
Copy link
Member

@mattkrick mattkrick commented May 10, 2024

Description

fix #9254

Supports uploading an XML file, which can then be used for the metadata URL

Demo

https://www.loom.com/share/23e34f4943ca4650803c9faad05bc488?sid=9d52a92c-093b-4b88-8bc3-8f37c31db1ca

@@ -61,7 +63,7 @@ const permissionMap: PermissionMap<Resolvers> = {
SAMLIdP: rateLimit({perMinute: 120, perHour: 3600})
},
Organization: {
saml: and(isViewerBillingLeaderSource, isOrgTierSource('enterprise'))
saml: and(isViewerBillingLeader('source.id'), isOrgTier('source.id', 'enterprise'))
Copy link
Member Author

Choose a reason for hiding this comment

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

curious on your thoughts about this new pattern.
I want to reuse rules as much as I can. For example, this rule requires an orgId to determine if the viewer is on the org. That orgId may exist in an arg, or in the source, and it could have any name. I think a dot notation path is the cleanest way to do this, but wonder what you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

I like it! Not having type safety is my main concern, but that's still probably safer that accidentally mixing isOrgTierSource for isOrgTier

@mattkrick mattkrick requested a review from nickoferrall May 10, 2024 20:48
@@ -61,7 +63,7 @@ const permissionMap: PermissionMap<Resolvers> = {
SAMLIdP: rateLimit({perMinute: 120, perHour: 3600})
},
Organization: {
saml: and(isViewerBillingLeaderSource, isOrgTierSource('enterprise'))
saml: and(isViewerBillingLeader('source.id'), isOrgTier('source.id', 'enterprise'))
Copy link
Contributor

Choose a reason for hiding this comment

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

I like it! Not having type safety is my main concern, but that's still probably safer that accidentally mixing isOrgTierSource for isOrgTier

@nickoferrall nickoferrall merged commit 5c40fcf into master May 20, 2024
7 checks passed
@nickoferrall nickoferrall deleted the fix/saml-upload branch May 20, 2024 17:43
@github-actions github-actions bot mentioned this pull request May 21, 2024
24 tasks
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.

SAML: Allow manually uploading the metadata as XML
2 participants