-
Notifications
You must be signed in to change notification settings - Fork 12.3k
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
Login: Allow custom name and icon for social providers #63297
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pr. Could you also update the sample.ini file with these values?
f08b681
to
75ed3e9
Compare
75ed3e9
to
dd74043
Compare
Sadly we cannot accept the pr, some providers has guidelines on how these buttons should look and feel. |
@kalleep I'm bit sad to hear this. The restrictions you mentioned are for marketplace related instances, but not for private instances. After merging the PR, the Sign button are still compliant for the mention guidelines. From grafana point of view, the Administrator of Grafana is now responsible for been complaint. Please, re-think about this. We have a Single Tenant Application on Azure AD and 2 Tenants to connect. I'm able to change the label of the generic oauth provider while I'm not able to change it for the azuread provider. Right now, we on our login screen
Which confuses or end users.I'm asking my self, why other solutions are less strict here and allows end-users to configure the label of the button, for example Gitlab, ArgoCD. Please @kalleep, re-think about this, if Grafana should enforce this on application level OR if its okay the move guideline compliance to grafana end-users like me. |
Hi @jkroepke, we discussed internally and have decided to allow merging the PR. You made some reasonable arguments implementing this change. Grafana also doesn't want to be in the position of enforcing the guidelines from the identity providers and rightly the responsibility should sit with end users / admins. We strive to provide consistent and powerful tooling to our users and recognise the value of this configuration option. |
@jkroepke before we can approve the pr, could you please also update the sample.ini file with the new options? |
dd74043
to
f98637b
Compare
@kalleep done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @jkroepke
What is this feature?
This PR allows the modification of social login providers
Details
Why do we need this feature?
Users may known to
Login with company IDP
orLogin with Azure AD
insteadLogin with Microsoft
Who is this feature for?
End Users
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Okta social provider already supports this. The field name and icon is already availible for all social providers, but the frontend component does not care about it.