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: Add UI for configuring SAML in Flagsmith #4055

Merged
merged 15 commits into from
Jun 14, 2024

Conversation

novakzaballa
Copy link
Contributor

Thanks for submitting a PR! Please check the boxes below:

  • I have run pre-commit to check linting
  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

  • Add UI for configuring SAML in Flagsmith wrapped with the saml_configuration flag.

How did you test this code?

  • Use an organisation with an enterprise plan
  • Go to Organisation Settings -> SAML tab
  • Create a SAML configuration

Copy link

vercel bot commented May 29, 2024

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

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2024 6:56am
2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
flagsmith-frontend-preview ⬜️ Ignored (Inspect) Visit Preview Jun 12, 2024 6:56am
flagsmith-frontend-staging ⬜️ Ignored (Inspect) Visit Preview Jun 12, 2024 6:56am

@github-actions github-actions bot added the front-end Issue related to the React Front End Dashboard label May 29, 2024
Copy link
Contributor

github-actions bot commented May 29, 2024

Uffizzi Preview deployment-52416 was deleted.

@novakzaballa novakzaballa requested a review from kyle-ssg May 30, 2024 00:09
@matthewelwell
Copy link
Contributor

@novakzaballa I've fixed a couple of minor issues here, but there are some fundamental issues that need resolving. We will, for example, need to add a new endpoint in the SAML repository (which I will work on now).

@matthewelwell
Copy link
Contributor

matthewelwell commented May 30, 2024

I've added a new list endpoint in the PR here which we will need to update this code to use. It's also worth noting that we allow multiple SamlConfiguration objects per organisation, so this PR will need to be updated to handle that case - it seems like it only supports a single configuration at the moment.

Some other changes that will need to be made:

  1. We should validate the name attribute as it needs to be a valid URL path parameter. This means we should restrict the input to only alphanumeric + - and _.
  2. We need to make sure that we handle errors from the API since the name field is unique across all organisations in the platform.
  3. We should add a tooltip explaining what the name field is: "An alphanumeric string to uniquely identify the SAML configuration. You will use this as the 'Organisation name' when logging in via the Flagsmith login page. Must only include alphanumeric characters or hyphens / underscores."
  4. The SamlConfiguration endpoints use the name field as the lookup field whereas this PR seems to be trying to use the numeric id field after creation.
  5. The IdP metadata can only be XML, and will be multiline. Can we update the input accordingly? Can the syntax validation be locked to XML perhaps?

We should also note that (in the near future), we will want to also be able to manage a list of attribute mappings (which consist of a string input, and a select box (values TBC). So we should consider that when designing the UI to allow for multiple SamlConfiguration objects.

Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

I've tested the UI aspects and all seems good now.

Added a few comments for some typos / improvements to the documentation.

@github-actions github-actions bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Jun 11, 2024
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

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

Approved with one more minor typo (which I'll fix now)

@github-actions github-actions bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Jun 12, 2024
@github-actions github-actions bot added docs Documentation updates feature New feature or request and removed feature New feature or request docs Documentation updates labels Jun 12, 2024
@novakzaballa novakzaballa added this pull request to the merge queue Jun 14, 2024
Merged via the queue into main with commit d2c2aba Jun 14, 2024
18 checks passed
@novakzaballa novakzaballa deleted the feat/add-ui-for-configuring-saml-in-flagsmith branch June 14, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request front-end Issue related to the React Front End Dashboard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add UI for configuring SAML via Flagsmith dashboard
2 participants