Skip to content

Conversation

tuunit
Copy link
Member

@tuunit tuunit commented Aug 27, 2023

Description and Motivation

The OIDC provider shouldn't be treated as anything specially where as all the other providers have their internal scope logic defined in the specific provider files the scope logic for oidc and in extension the keycloak-oidc provider is located in the providers.go.

To align the OIDC and Keycloak OIDC provider I moved the logic to the oidc.go provider file.

How Has This Been Tested?

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

Replaces PR: #1989
Fixes:#2222

@tuunit tuunit requested a review from a team as a code owner August 27, 2023 10:29
@kvanzuijlen
Copy link
Member

kvanzuijlen commented Sep 7, 2023

@tuunit please add fixes #2222 and resolve the conflicts. @JoelSpeed can you look at this PR so we can get 7.5.1 released ASAP?

@JoelSpeed JoelSpeed added the bug label Sep 7, 2023
@tuunit tuunit force-pushed the bugfix/move-oidc-scope-logic branch from dced5bd to 6f6039c Compare September 8, 2023 05:35
@tuunit
Copy link
Member Author

tuunit commented Sep 8, 2023

@JoelSpeed Sorry that I didn't notice this was missed in the release :/
I updated the CHANGELOG

Copy link
Member

@kvanzuijlen kvanzuijlen left a comment

Choose a reason for hiding this comment

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

LGTM

@tuunit
Copy link
Member Author

tuunit commented Sep 8, 2023

#2222

@kvanzuijlen
Copy link
Member

@tuunit I think it has to be included in the description with something like fixes in front of it

@JoelSpeed JoelSpeed merged commit 9f06dc8 into oauth2-proxy:master Sep 8, 2023
@tuunit tuunit deleted the bugfix/move-oidc-scope-logic branch December 17, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants