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

SSO: Display provider list #78472

Merged
merged 26 commits into from
Nov 29, 2023
Merged

SSO: Display provider list #78472

merged 26 commits into from
Nov 29, 2023

Conversation

Clarity-89
Copy link
Contributor

@Clarity-89 Clarity-89 commented Nov 21, 2023 •
edited
Loading

What is this feature?

Display the list of the new SSO providers using a grid list.

Why do we need this feature?

Part of the SSO UI project.

Which issue(s) does this PR fix?:

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

Special notes for your reviewer:
The feature is behind the ssoSettingsApi feature toggle.

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.

@Clarity-89 Clarity-89 self-assigned this Nov 21, 2023
@grafana-delivery-bot grafana-delivery-bot bot added this to the 10.3.x milestone Nov 21, 2023
@Clarity-89 Clarity-89 marked this pull request as ready for review November 22, 2023 11:39
@Clarity-89 Clarity-89 requested review from grafanabot and a team as code owners November 22, 2023 11:39
mgyongyosi
mgyongyosi reviewed Nov 27, 2023
View reviewed changes
Copy link
Contributor

@mgyongyosi mgyongyosi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks, it looks good! I found a small thing related to permissions: right now the Authentication page is only available for Grafana Enterprise, however the OAuth providers configuration is an OSS feature. Could you please fix this? I'm happy to assist!

@Clarity-89 Clarity-89 requested review from a team as code owners November 28, 2023 05:51
@Clarity-89 Clarity-89 requested review from tskarhed, L-M-K-B, mildwonkey, nikimanoledaki and suntala and removed request for a team November 28, 2023 05:51
@Clarity-89
Copy link
Contributor Author

Thanks, it looks good! I found a small thing related to permissions: right now the Authentication page is only available for Grafana Enterprise, however the OAuth providers configuration is an OSS feature. Could you please fix this? I'm happy to assist!

Good catch! Looks like it's available in OSS but only if the user has SAML or LDAP configured. Now the auth view should be also shown if the feature toggle is enabled.

mgyongyosi
mgyongyosi approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@mgyongyosi mgyongyosi left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks! We are going to handle the RBAC based filtering in a follow up PR (listing only the OAuth providers that the user has write permission to + listing only the configurable providers (this hasn't been implemented yet)).

@Clarity-89 Clarity-89 merged commit 1141dd6 into main Nov 29, 2023
16 checks passed
@Clarity-89 Clarity-89 deleted the sso/provider-list branch November 29, 2023 17:09
@aangelisc aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023
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

@grafanabot grafanabot Awaiting requested review from grafanabot grafanabot is a code owner

@colin-stuart colin-stuart Awaiting requested review from colin-stuart

@tskarhed tskarhed Awaiting requested review from tskarhed tskarhed is a code owner automatically assigned from grafana/grafana-frontend-platform

@L-M-K-B L-M-K-B Awaiting requested review from L-M-K-B L-M-K-B is a code owner automatically assigned from grafana/grafana-frontend-platform

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

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

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

Assignees

@Clarity-89 Clarity-89

Labels
add to changelog area/backend area/frontend no-backport Skip backport of PR
Projects
None yet
Milestone
10.2.3
Development

Successfully merging this pull request may close these issues.
3 participants
@Clarity-89 @mgyongyosi @aangelisc