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.

Sign up for GitHub

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

Jump to bottom

samlsettings: api integration #84300

Merged
merged 39 commits into from Mar 25, 2024
Merged

samlsettings: api integration #84300

merged 39 commits into from Mar 25, 2024

Conversation

linoman
Copy link
Contributor

@linoman linoman commented Mar 12, 2024 •
edited

What is this feature?

This PR registers the sso settings saml strategy; this way we can update and retrieve saml sso settings from the sso endpoints. This PR also renames the functions of the old saml strategy interface, since they're not compatible with the new sso settings interface.

Why do we need this feature?

This PR allows to interact and save the saml settings via the sso settings api

Who is this feature for?

IAM

Which issue(s) does this PR fix?:

Fixes https://github.com/grafana/identity-access-team/issues/594

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.0.x milestone Mar 12, 2024
@grafanabot
Copy link
Contributor

❌ Failed to run Playwright plugin e2e tests.

Click here to browse the Playwright report and trace viewer.
For information on how to run Playwright tests locally, refer to the Developer guide.

@linoman linoman self-assigned this Mar 12, 2024
@grafanabot
Copy link
Contributor

❌ Failed to run Playwright plugin e2e tests.

Click here to browse the Playwright report and trace viewer.
For information on how to run Playwright tests locally, refer to the Developer guide.

@linoman linoman added no-backport Skip backport of PR no-changelog Skip including change in changelog/release notes labels Mar 13, 2024
@mgyongyosi mgyongyosi requested a review from dmihai March 13, 2024 10:00
@linoman linoman requested review from mgyongyosi and a team March 13, 2024 16:47
@linoman linoman marked this pull request as ready for review March 13, 2024 16:47
@linoman linoman requested a review from a team as a code owner March 13, 2024 16:47
@mgyongyosi mgyongyosi requested a review from dmihai March 18, 2024 14:25
@linoman
Copy link
Contributor Author

linoman commented Mar 18, 2024

/deploy-to-hg

@ephemeral-instances-bot
Copy link

  • Preparing your instance. A comment containing your instance's url will be added to this PR when the instance is ready.
  • Your instance will be ready in ~10 minutes.
  • Check the GitHub actions tab to follow the workflow progress
  • Slack channel: #proj-ephemeral-hg-instances
  • Building instance with linoman/samlsettings_api_integration oss branch and main enterprise branch. How to choose a branch

@ephemeral-instances-bot

This comment was marked as resolved.

Sign in to view
@linoman
Copy link
Contributor Author

linoman commented Mar 18, 2024

/deploy-to-hg --enterprise-ref linoman/samlsettings_api_integration

@ephemeral-instances-bot

This comment was marked as resolved.

Sign in to view
@ephemeral-instances-bot
Copy link

dmihai
dmihai reviewed Mar 18, 2024
View reviewed changes
Copy link
Contributor

@dmihai dmihai left a comment

Choose a reason for hiding this comment

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

We might also need some more unit tests for the SAML provider that you introduced to the SSO Settings service. But we can also add them in a separate PR if you want.

pkg/services/ssosettings/ssosettingsimpl/service.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/ssosettingsimpl/service.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/ssosettingsimpl/service_test.go Outdated Show resolved Hide resolved
@linoman linoman merged commit fc205db into main Mar 25, 2024
12 checks passed
@linoman linoman deleted the linoman/samlsettings_api_integration branch March 25, 2024 09:54
@ashharrison90 ashharrison90 modified the milestones: 11.0.x, 11.1.x Mar 25, 2024
@ashharrison90 ashharrison90 modified the milestones: 11.1.x, 11.0.0-preview Apr 15, 2024
@ashharrison90 ashharrison90 added product-approved Pull requests that are approved by product/managers and are allowed to be backported backport v11.0.x Mark PR for automatic backport to v11.0.x and removed no-backport Skip backport of PR labels Apr 15, 2024
@grafana-delivery-bot grafana-delivery-bot bot mentioned this pull request Apr 15, 2024
Merged
3 tasks
grafana-delivery-bot bot pushed a commit that referenced this pull request Apr 15, 2024
@linoman @grafana-delivery-bot
samlsettings: api integration (#84300)
0f906d3 
* add strategy and tests

* use settings provider service and remove multiple providers strategy

* Move SAML strategy to ssosettings service

* Update codeowners file

* reload from settings provider

* add saml as configurable provider

* Add new SAML strategy

* rename old saml settings interface

* update saml string references

* use OSS license

* validate saml provider depends on license for List

* add tests for list rendering including saml

* change the licensing validation to service init

* replace service struct for provider

(cherry picked from commit fc205db)
ashharrison90 pushed a commit that referenced this pull request Apr 15, 2024
@grafana-delivery-bot @linoman
[v11.0.x] samlsettings: api integration (#86109)
6d0e1be 
samlsettings: api integration (#84300)

* add strategy and tests

* use settings provider service and remove multiple providers strategy

* Move SAML strategy to ssosettings service

* Update codeowners file

* reload from settings provider

* add saml as configurable provider

* Add new SAML strategy

* rename old saml settings interface

* update saml string references

* use OSS license

* validate saml provider depends on license for List

* add tests for list rendering including saml

* change the licensing validation to service init

* replace service struct for provider

(cherry picked from commit fc205db)

Co-authored-by: linoman <2051016+linoman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgyongyosi mgyongyosi mgyongyosi approved these changes

@dmihai dmihai dmihai approved these changes

@papagian papagian Awaiting requested review from papagian papagian was automatically assigned from grafana/backend-platform

@diegommm diegommm Awaiting requested review from diegommm diegommm was automatically assigned from grafana/backend-platform

@suntala suntala Awaiting requested review from suntala suntala was automatically assigned from grafana/backend-platform

Assignees

@linoman linoman

Labels
area/auth/saml area/backend backport v11.0.x Mark PR for automatic backport to v11.0.x no-changelog Skip including change in changelog/release notes product-approved Pull requests that are approved by product/managers and are allowed to be backported
Projects
None yet
Milestone
11.0.0-preview
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@linoman @grafanabot @mgyongyosi @dmihai @ashharrison90