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

Jump to bottom

Auth: Allow admins to manually change external user's role if oauth_skip_org_role_update_sync or saml skip_org_role_sync is enabled #55182

Merged
merged 5 commits into from Sep 15, 2022
Merged

Auth: Allow admins to manually change external user's role if oauth_skip_org_role_update_sync or saml skip_org_role_sync is enabled #55182

merged 5 commits into from Sep 15, 2022

Conversation

Jguer
Copy link
Contributor

@Jguer Jguer commented Sep 14, 2022 •
edited by gamab

What this PR does / why we need it:

Auth:

  • Allow admins to change OAuth user role if it's not synced
  • Allow admins to change SAML user role if it's not synced

Fixes #54688

Special notes for your reviewer:

@Jguer @gamab
Auth: Allow admins to change oauth user info it it's not synced.
2db2bdc 
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
@Jguer Jguer added type/bug add to changelog backport v9.1.x Bot will automatically open backport PR labels Sep 14, 2022
@Jguer Jguer added this to the 9.2.0 milestone Sep 14, 2022
@Jguer Jguer self-assigned this Sep 14, 2022
@Jguer Jguer requested review from a team as code owners September 14, 2022 16:19
@Jguer Jguer requested a review from a team September 14, 2022 16:19
@Jguer Jguer requested a review from a team as a code owner September 14, 2022 16:19
@Jguer Jguer requested review from JoaoSilvaGrafana, jackw, zoltanbedi, mdvictor, sakjur, idafurjes, yangkb09, gamab and a team and removed request for a team September 14, 2022 16:19
@Jguer
Copy link
Contributor Author

Jguer commented Sep 14, 2022

@Clarity-89 probably there is a way to do this more in line with grafana's frontend code style. Would appreciate some guidance on it

@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/33467

@Jguer Jguer changed the title Auth: Allow admins to change oauth user role it it's not synced Auth: Allow admins to manually change oauth user role if skip_oauth_sync is enabled Sep 15, 2022
@Jguer Jguer changed the title Auth: Allow admins to manually change oauth user role if skip_oauth_sync is enabled Auth: Allow admins to manually change oauth user role if oauth_skip_org_role_update_sync is enabled Sep 15, 2022
@gamab @Jguer
SAML: Add option to skip org role sync (#55230)
1b62507 
* SAML: Add option to skip org role sync

* Modify frontend accordingly

* Remove update from config option name

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Remove update from config option name

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Fix typo

Co-authored-by: Jguer <joao.guerreiro@grafana.com>
@gamab gamab requested a review from a team as a code owner September 15, 2022 15:33
@gamab gamab requested review from Eve832 and GrafanaWriter and removed request for a team September 15, 2022 15:33
@Jguer Jguer enabled auto-merge (squash) September 15, 2022 15:38
@gamab gamab changed the title Auth: Allow admins to manually change oauth user role if oauth_skip_org_role_update_sync is enabled Auth: Allow admins to manually change external user's role if oauth_skip_org_role_update_sync or saml_skip_org_role_sync is enabled Sep 15, 2022
@gamab gamab changed the title Auth: Allow admins to manually change external user's role if oauth_skip_org_role_update_sync or saml_skip_org_role_sync is enabled Auth: Allow admins to manually change external user's role if oauth_skip_org_role_update_sync or saml skip_org_role_sync is enabled Sep 15, 2022
@grafanabot
Copy link
Contributor

Drone build failed: https://drone.grafana.net/grafana/grafana-enterprise/33646

@Jguer Jguer merged commit 3e2e9f9 into main Sep 15, 2022
@Jguer Jguer deleted the jguer-gamab/respect-skip-oauth branch September 15, 2022 16:06
@grafanabot
Copy link
Contributor

The backport to v9.1.x failed:

could not import config from "/home/runner/work/grafana/grafana/grafana/.betterer.ts". 😔

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-55182-to-v9.1.x origin/v9.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x 3e2e9f93b96ad565607a30387cd230ef87a4e985
# Push it to GitHub
git push --set-upstream origin backport-55182-to-v9.1.x
git switch main
# Remove the local backport branch
git branch -D backport-55182-to-v9.1.x

Then, create a pull request where the base branch is v9.1.x and the compare/head branch is backport-55182-to-v9.1.x.

Jguer added a commit that referenced this pull request Sep 16, 2022
@Jguer
Auth: Allow admins to manually change oauth user role if `oauth_skip_…
4ed7af9 
…org_role_update_sync` is enabled (#55182)

* Auth: Allow admins to change oauth user info it it's not synced.

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* Update public/app/features/admin/UserAdminPage.tsx

Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>

* Add missing import

* Simplify init

Co-authored-by: Josh Hunt <joshhunt@users.noreply.github.com>
Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>

* SAML: Add option to skip org role sync (#55230)

* SAML: Add option to skip org role sync

* Modify frontend accordingly

* Remove update from config option name

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Remove update from config option name

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Fix typo

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>
Co-authored-by: Josh Hunt <joshhunt@users.noreply.github.com>
(cherry picked from commit 3e2e9f9)
@Jguer Jguer mentioned this pull request Sep 16, 2022
Merged
Jguer added a commit that referenced this pull request Sep 19, 2022
@Jguer
Backport: Revert changes to oauth behavior to v9.1.x (#55302)
5f51a14 
* OAuth: Allow assigning Server Admin (#54780)

* extract errors to errors file

* implement oauth server admin assignment

* add server admin tests

* deduplicate autoAssignOrgRole

* deduplicate strict setting

* deduplicate strict setting

* add support for generic oauth

* add role attribute strict support for generic oauth

* add support for github/gitlab

* assignGrafanaAdmin option is here to stay

* unify similar errors

* add config option

* add okta server admin mapping

* remove never used Company attribute

* unify generic oauth role extract with other methods

* case insensitive role match as in azure

* add ini settings

* add server admin to devenv

* remove duplicate fields

* add documentation to oauth

* fix titlecase test

* implement doc feedback

(cherry picked from commit ef24587)

* Auth: Restore legacy behavior and add deprecation notice for empty org role in oauth (#55118)

* Auth: Add deprecation notice for empty org role

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* fix recasts

* fix azure tests missing logger

* Adding test to gitlab oauth

* Covering more cases

* Cover more options

* Add role attributestrict check fail

* Adding one more edge case test

* Using legacy for gitlab

* Yet another edge case YAEC

* Reverting github oauth to legacy

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Not using token

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Nit.

* Adding warning in docs

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* add warning to generic oauth

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Be more precise

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Adding warning to github oauth

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Adding warning to gitlab oauth

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Adding warning to okta oauth

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Add docs about mapping to AzureAD

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Clarify oauth_skip_org_role_update_sync

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Nit.

* Nit on Azure AD

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Reorder docs index

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Fix typo

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>
(cherry picked from commit 00e7324)

* Auth: Allow admins to manually change oauth user role if `oauth_skip_org_role_update_sync` is enabled  (#55182)

* Auth: Allow admins to change oauth user info it it's not synced.

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* Update public/app/features/admin/UserAdminPage.tsx

Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>

* Add missing import

* Simplify init

Co-authored-by: Josh Hunt <joshhunt@users.noreply.github.com>
Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>

* SAML: Add option to skip org role sync (#55230)

* SAML: Add option to skip org role sync

* Modify frontend accordingly

* Remove update from config option name

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Remove update from config option name

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

* Fix typo

Co-authored-by: Jguer <joao.guerreiro@grafana.com>

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>
Co-authored-by: Josh Hunt <joshhunt@users.noreply.github.com>
(cherry picked from commit 3e2e9f9)

* Update gitlab_oauth_test.go

* Update gitlab_oauth_test.go
@dsotirakis dsotirakis modified the milestones: 9.2.0, 9.2.0-beta1 Sep 22, 2022
This was referenced Sep 22, 2022
Closed
Closed
Closed
@eti
Copy link

eti commented Sep 26, 2022

I cannot find any evidence that this change will fix also LDAP auth. How does Grafana expect to add users to different organizations and grant them roles there?

@mellieA mellieA mentioned this pull request Sep 27, 2022
Closed
@Tobewont Tobewont mentioned this pull request Nov 23, 2022
Closed
@tobhv tobhv mentioned this pull request Nov 30, 2022
Closed
@mellieA mellieA mentioned this pull request Jan 31, 2023
Closed
@dm3ch dm3ch mentioned this pull request Feb 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshhunt joshhunt joshhunt left review comments

@gamab gamab gamab approved these changes

@Clarity-89 Clarity-89 Clarity-89 approved these changes

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

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

@zoltanbedi zoltanbedi Awaiting requested review from zoltanbedi

@mdvictor mdvictor Awaiting requested review from mdvictor

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

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

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

@Eve832 Eve832 Awaiting requested review from Eve832 Eve832 was automatically assigned from grafana/docs-squad

@GrafanaWriter GrafanaWriter Awaiting requested review from GrafanaWriter GrafanaWriter was automatically assigned from grafana/docs-squad

Assignees

@Jguer Jguer

Labels
add to changelog area/backend area/frontend backport v9.1.x Bot will automatically open backport PR enterprise-failed type/bug type/docs
Projects
None yet
Milestone
9.2.0-beta1
Development

Successfully merging this pull request may close these issues.

Auth: Take into account oauth_skip_org_role_update_sync for locking down UI
7 participants
@Jguer @grafanabot @eti @joshhunt @gamab @Clarity-89 @dsotirakis