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

Auth: Load ini/env vars settings in the fallback strategy #78495

Merged
merged 21 commits into from
Dec 1, 2023
Merged

Auth: Load ini/env vars settings in the fallback strategy #78495

merged 21 commits into from
Dec 1, 2023

Conversation

mgyongyosi
Copy link
Contributor

@mgyongyosi mgyongyosi commented Nov 21, 2023 •
edited

What is this feature?
Default settings are loaded for the separate providers for replacing the defaults.ini file + OAuthStrategy loads the required settings for the OAuth providers.

Why do we need this feature?
To have backward compatibility with the ini/env vars config for the OAuth providers.

Who is this feature 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.

@mgyongyosi mgyongyosi added this to the 10.3.x milestone Nov 21, 2023
@mgyongyosi mgyongyosi marked this pull request as ready for review November 27, 2023 12:34
@mgyongyosi mgyongyosi requested a review from a team as a code owner November 27, 2023 12:34
@mgyongyosi mgyongyosi changed the title [WIP] Auth: Load default setting in the fallback strategy for loading OAuth settings Auth: Load ini/env vars settings in the fallback strategy Nov 27, 2023
@mgyongyosi mgyongyosi marked this pull request as draft November 27, 2023 12:47
@mgyongyosi mgyongyosi marked this pull request as ready for review November 30, 2023 13:29
@mgyongyosi mgyongyosi requested a review from a team as a code owner November 30, 2023 13:29
@mgyongyosi mgyongyosi requested review from zserge, mildwonkey, nikimanoledaki and kalleep and removed request for a team November 30, 2023 13:29
mgyongyosi
mgyongyosi commented Nov 30, 2023
View reviewed changes
pkg/setting/setting.go Outdated Show resolved Hide resolved
kalleep
kalleep reviewed Nov 30, 2023
View reviewed changes
pkg/login/social/generic_oauth.go Outdated Show resolved Hide resolved
pkg/login/social/github_oauth.go Outdated Show resolved Hide resolved
pkg/login/social/grafana_com_oauth.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/strategies/oauth_strategy.go Outdated Show resolved Hide resolved
@mgyongyosi mgyongyosi force-pushed the mgyongyosi/load-defaults-oauth-strategy branch from bd1d1f6 to 096854f Compare November 30, 2023 18:08
@mgyongyosi mgyongyosi force-pushed the mgyongyosi/load-defaults-oauth-strategy branch from 096854f to a7bc009 Compare November 30, 2023 18:23
gamab
gamab approved these changes Dec 1, 2023
View reviewed changes
Copy link
Contributor

@gamab gamab left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/services/ssosettings/strategies/oauth_strategy.go Show resolved Hide resolved
pkg/services/ssosettings/strategies/oauth_strategy.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/strategies/oauth_strategy.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/strategies/oauth_strategy_test.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/strategies/oauth_strategy_test.go Outdated Show resolved Hide resolved
mgyongyosi and others added 2 commits December 1, 2023 10:05
@mgyongyosi
Changes requested by @gamab
f9032ab
@mgyongyosi @gamab
Update pkg/services/ssosettings/strategies/oauth_strategy_test.go
63656e3 
Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
@mgyongyosi mgyongyosi requested a review from gamab December 1, 2023 10:11
@mgyongyosi mgyongyosi force-pushed the mgyongyosi/load-defaults-oauth-strategy branch from cacfd56 to f22fe93 Compare December 1, 2023 10:58
gamab
gamab approved these changes Dec 1, 2023
View reviewed changes
pkg/services/ssosettings/ssosettings.go Outdated Show resolved Hide resolved
pkg/services/ssosettings/strategies/oauth_strategy.go Show resolved Hide resolved
@mgyongyosi mgyongyosi force-pushed the mgyongyosi/load-defaults-oauth-strategy branch from 24d26ba to e310f4e Compare December 1, 2023 12:01
@mgyongyosi mgyongyosi merged commit d3a269a into main Dec 1, 2023
13 checks passed
@mgyongyosi mgyongyosi deleted the mgyongyosi/load-defaults-oauth-strategy branch December 1, 2023 14:35
@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

@gamab gamab gamab approved these changes

@kalleep kalleep kalleep approved these changes

@dmihai dmihai Awaiting requested review from dmihai

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

@zserge zserge Awaiting requested review from zserge zserge was automatically assigned from grafana/backend-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

Assignees
No one assigned
Labels
add to changelog area/auth area/backend no-backport Skip backport of PR
Projects
None yet
Milestone
10.2.3
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@mgyongyosi @gamab @kalleep @aangelisc