Generic OIDC group scope is not sent in the request formed by the UI #13514
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…unction works as expected
cnotv
added a commit
to cnotv/dashboard
that referenced
this pull request
Mar 11, 2025
…ancher#13514) * Add basic store auth tests for redirect * Define mixin tests * Correct tests to cover all the cases correctly and ensure that this function works as expected * Add scopes to the redirectTo request when testing the model * Add unit tests for action test * Add comment to save auth config method * Correct unit tests * Correct test to avoid the same term to be used for different parts * Correct key value from scope to scopes to match existing implementation * Update tests to use scopesJoinChar * Add further tests for merging default scopes and UI scopes * Add joinStringList function * Correct scopes merging issues
Merged
7 tasks
cnotv
added a commit
to cnotv/dashboard
that referenced
this pull request
Mar 11, 2025
…ancher#13514) * Add basic store auth tests for redirect * Define mixin tests * Correct tests to cover all the cases correctly and ensure that this function works as expected * Add scopes to the redirectTo request when testing the model * Add unit tests for action test * Add comment to save auth config method * Correct unit tests * Correct test to avoid the same term to be used for different parts * Correct key value from scope to scopes to match existing implementation * Update tests to use scopesJoinChar * Add further tests for merging default scopes and UI scopes * Add joinStringList function * Correct scopes merging issues
Merged
7 tasks
cnotv
added a commit
to cnotv/dashboard
that referenced
this pull request
Mar 11, 2025
…ancher#13514) * Add basic store auth tests for redirect * Define mixin tests * Correct tests to cover all the cases correctly and ensure that this function works as expected * Add scopes to the redirectTo request when testing the model * Add unit tests for action test * Add comment to save auth config method * Correct unit tests * Correct test to avoid the same term to be used for different parts * Correct key value from scope to scopes to match existing implementation * Update tests to use scopesJoinChar * Add further tests for merging default scopes and UI scopes * Add joinStringList function * Correct scopes merging issues
cnotv
added a commit
that referenced
this pull request
Mar 11, 2025
…13514) (#13679) * Add basic store auth tests for redirect * Define mixin tests * Correct tests to cover all the cases correctly and ensure that this function works as expected * Add scopes to the redirectTo request when testing the model * Add unit tests for action test * Add comment to save auth config method * Correct unit tests * Correct test to avoid the same term to be used for different parts * Correct key value from scope to scopes to match existing implementation * Update tests to use scopesJoinChar * Add further tests for merging default scopes and UI scopes * Add joinStringList function * Correct scopes merging issues
cnotv
added a commit
that referenced
this pull request
Mar 11, 2025
… formed by the UI (#13681) * Generic OIDC group scope is not sent in the request formed by the UI (#13514) * Add basic store auth tests for redirect * Define mixin tests * Correct tests to cover all the cases correctly and ensure that this function works as expected * Add scopes to the redirectTo request when testing the model * Add unit tests for action test * Add comment to save auth config method * Correct unit tests * Correct test to avoid the same term to be used for different parts * Correct key value from scope to scopes to match existing implementation * Update tests to use scopesJoinChar * Add further tests for merging default scopes and UI scopes * Add joinStringList function * Correct scopes merging issues * Remove unit tests incompatible with this backported version
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #12477
Occurred changes and/or fixed issues
Included scopes in the Keycloack pop-up as indicated in the issue.
Technical notes summary
addObjects()with newjoinStringList(), including unit testsredirectToaction to generate redirect URL to auth provider pop up with all the involved casestestaction to pass all the required options to compose the redirect URLauth-configmixin used in the view to submit the auth configurationconcatStrings()Side notes as report:
It seems like
scopewas not added to the whole logic after passing the list from the UI. The function where it gets added has not been changed for 5 years and was not working, so nothing about this feature has been used before.The scopes seem to be in a format
['a b c']with a single item in the array, but a refactor has been pushed to a separate issue (#13457) since this is way out of scope.In the store logic it's also misspelled to the plural as
scopes, which may leads to confusion while reviewing.Areas or cases that should be tested
As by reported issue.
Areas which could experience regressions
Screenshot/Video
Video for this PR is not possible due to internal information, however a new page with the correct scope (as per test) is generated:
Checklist