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: Move LDAP debug to Authentication menu #71285

Merged
merged 2 commits into from Jul 12, 2023
Merged

Conversation

Jguer
Copy link
Contributor

@Jguer Jguer commented Jul 10, 2023

What is this feature?

  • Move LDAP debug menu to an authentication card

image

When LDAP is disabled card won't show

image

This keeps the current behavior until extra LDAP configuration is added

Why do we need this feature?

[Add a description of the problem the feature is trying to solve.]

Who is this feature for?

[Add information on what kind of user the feature is for.]

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.

@Jguer Jguer added add to changelog no-backport Skip backport of PR labels Jul 10, 2023
@Jguer Jguer requested review from a team as code owners July 10, 2023 13:30
@Jguer Jguer self-assigned this Jul 10, 2023
@Jguer Jguer requested a review from a team as a code owner July 10, 2023 13:30
@Jguer Jguer requested review from L-M-K-B, eledobleefe, linoman, IevaVasiljeva, sakjur, papagian, zserge, alexanderzobnin, eleijonmarck and a team and removed request for a team July 10, 2023 13:30
@Jguer Jguer added this to the 10.1.x milestone Jul 10, 2023
ac.EvalPermission(ac.ActionSettingsWrite, ac.ScopeSettingsSAML),
ac.EvalPermission(ac.ActionSettingsRead, ac.ScopeSettingsSAML),
), ac.EvalPermission(ac.ActionLDAPStatusRead))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

one issue that this PR raises is to have the ScopeSettingsLDAP ready. could we add that as a follow up issue ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 you want me to do it?

Copy link
Contributor

@eleijonmarck eleijonmarck left a comment

Choose a reason for hiding this comment

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

LGTM, this could be merged before a big refactor of the UI from this PR. Ideally we would merge this PR first and then we could merged the conflicts into the bigger PR with the refactored states

oss - #71200
enterprise - https://github.com/grafana/grafana-enterprise/pull/5398

@Jguer
Copy link
Contributor Author

Jguer commented Jul 12, 2023

👍 I do agree it requires some frontend rework, I'll trust you on that and can review once rebased

@Jguer Jguer merged commit 9b22342 into main Jul 12, 2023
21 checks passed
@Jguer Jguer deleted the jguer/move-ldap-debug branch July 12, 2023 16:15
polibb pushed a commit that referenced this pull request Jul 14, 2023
* move LDAP page to Authentication

* tweak Auth menu showing permissions
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
@mhollstein
Copy link

mhollstein commented Aug 25, 2023

I don't see the Authentication under Administration menu, so I'm not be able to use LDAP Test user mapping.
Grafana 10.1.0 Docker Image: docker pull grafana/grafana:10.1.0

@mari-arondeus
Copy link

I don't see the Authentication under Administration menu, so I'm not be able to use LDAP Test user mapping. Grafana 10.1.0 Docker Image: docker pull grafana/grafana:10.1.0

Ditto. Maybe it only shows up if you have enterprise licensing so the SAML option would also be available? We're running Grafana Enterprise on-prem 10.1.0 without licensing, and the option is missing for us as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants