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

Auth: add the missing fields for all SSO providers #83813

Merged
merged 14 commits into from Mar 19, 2024
Merged

Conversation

dmihai
Copy link
Contributor

@dmihai dmihai commented Mar 4, 2024

What is this feature?

Add all config fields for the SSO providers in the UI.

Why do we need this feature?

For SSO Settings API.

Who is this feature for?

everybody

Which issue(s) does this PR fix?:

Fixes #

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.

@dmihai dmihai requested a review from a team March 4, 2024 10:13
@dmihai dmihai requested a review from a team as a code owner March 4, 2024 10:13
@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.0.x milestone Mar 4, 2024
fields: ['allowAssignGrafanaAdmin', 'roleAttributePath', 'roleAttributeStrict', 'skipOrgRoleSync'],
},
{
name: 'Extra security measures',
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add the validate_hd setting? #83229

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the new field validate_hd. Can you please check the description I added to it? https://github.com/grafana/grafana/blob/dmihai/sso-config-pages/public/app/features/auth-config/fields.tsx#L595-L600

@dmihai dmihai added the no-changelog Skip including change in changelog/release notes label Mar 6, 2024
@dmihai dmihai requested a review from mgyongyosi March 14, 2024 12:20
{
name: 'User mapping',
id: 'user',
fields: ['allowAssignGrafanaAdmin', 'roleAttributePath', 'roleAttributeStrict', 'skipOrgRoleSync'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the order of these in each of the new configurations. The order should be (based on generic_oauth): 'roleAttributePath', 'roleAttributeStrict','allowAssignGrafanaAdmin','skipOrgRoleSync'.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this for all providers.

'clientId',
'clientSecret',
'scopes',
'authStyle',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed.

'clientId',
'clientSecret',
'scopes',
'authStyle',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed.

@dmihai dmihai requested a review from mgyongyosi March 18, 2024 10:45
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.

LGTM! Thanks! 🚀

@dmihai dmihai merged commit 6febfdf into main Mar 19, 2024
14 checks passed
@dmihai dmihai deleted the dmihai/sso-config-pages branch March 19, 2024 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend no-changelog Skip including change in changelog/release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants