Skip to content

Fix GDM name display for google/generic oidc brokers#1369

Open
nooreldeenmansour wants to merge 1 commit intomainfrom
fix-gdm-name-preview
Open

Fix GDM name display for google/generic oidc brokers#1369
nooreldeenmansour wants to merge 1 commit intomainfrom
fix-gdm-name-preview

Conversation

@nooreldeenmansour
Copy link
Copy Markdown
Member

@nooreldeenmansour nooreldeenmansour commented Mar 26, 2026

Closes #1368

UDENG-9509

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes GDM user preview showing the email instead of the user’s full name for the google and generic OIDC brokers by aligning the parsed ID token claim with the standard OIDC name claim (and updating test mocks + goldens accordingly).

Changes:

  • Map GenericProvider’s display-name (GECOS) field to the OIDC standard name claim instead of the non-standard gecos claim.
  • Update the mock provider claims parsing and genericprovider unit test to use name.
  • Refresh broker golden outputs to reflect gecos being populated from the name claim.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
authd-oidc-brokers/internal/providers/genericprovider/genericprovider.go Parse display name from standard OIDC name claim for generic/google flows.
authd-oidc-brokers/internal/testutils/provider.go Align mock provider claim parsing with name so tests reflect real providers.
authd-oidc-brokers/internal/providers/genericprovider/genericprovider_test.go Update test claims to supply name and validate resulting user GECOS.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_password/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Successfully_authenticate_user_with_device_auth_and_newpassword/second_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Owner_extra_groups_configured_but_user_does_not_become_owner/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Owner_extra_groups_configured/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Extra_groups_configured/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_qrcode_reacquires_token/second_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_when_provider_supports_device_registration/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_when_provider_authentication_is_forced/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_refreshes_groups/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_refreshes_expired_token/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_password_keeps_old_groups_if_fetching_groups_fails/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_with_device_auth_when_provider_supports_device_registration/second_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_when_the_auth_data_secret_field_uses_the_old_name/first_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestIsAuthenticated/Authenticating_still_allowed_if_token_is_missing_scopes/second_call Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_first_then_second_starts_and_first_finishes/second_auth Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_first_then_second_starts_and_first_finishes/first_auth Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_first_but_second_finishes_first_and_is_registered_as_the_owner/second_auth Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_first_but_second_finishes_first/second_auth Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_first_but_second_finishes_first/first_auth Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_and_finishes_before_second/second_auth Golden update: gecos now reflects display name from name claim.
authd-oidc-brokers/internal/broker/testdata/golden/TestConcurrentIsAuthenticated/First_auth_starts_and_finishes_before_second/first_auth Golden update: gecos now reflects display name from name claim.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.08%. Comparing base (6f0c33d) to head (f20b247).
⚠️ Report is 32 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1369      +/-   ##
==========================================
- Coverage   86.24%   80.08%   -6.17%     
==========================================
  Files          99       20      -79     
  Lines        6690      984    -5706     
  Branches      111        0     -111     
==========================================
- Hits         5770      788    -4982     
+ Misses        860      196     -664     
+ Partials       60        0      -60     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nooreldeenmansour nooreldeenmansour changed the title Fix broken name display for google/generic broker for GDM user preview Fix GDM name preview for google/generic oidc brokers Mar 26, 2026
@nooreldeenmansour nooreldeenmansour changed the title Fix GDM name preview for google/generic oidc brokers Fix GDM name display for google/generic oidc brokers Apr 1, 2026
Copy link
Copy Markdown
Contributor

@adombeck adombeck left a comment

Choose a reason for hiding this comment

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

The changes look good to me, but please squash the commit which updates the tests, so that the tests pass for each commit (for better git bisect results).

@adombeck
Copy link
Copy Markdown
Contributor

adombeck commented Apr 2, 2026

How about we mention in the AGENTS.md that we want the tests to pass on each commit (some call that "atomic commits" but there are other definitions)?

tests: update golden files and fixtures for GDM full name display fix
@nooreldeenmansour
Copy link
Copy Markdown
Member Author

@adombeck squashed the test commit.
And regarding the AGENTS.md suggestion, it might be a good idea. could be worth revisiting the AGENTS.md file as well.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Google and generic OIDC brokers show email instead of display name in GDM

3 participants