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

fix: Use name if preferredUsername is empty #148

Merged
merged 2 commits into from
Feb 24, 2021
Merged

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Feb 23, 2021

TL;DR

Use name from /me response if preferred_username is null.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Not all OIdP providers fill in all user info fields. Google, we found, uses the name field but not the preferred_username field.

Tracking Issue

flyteorg/flyte#657

@@ -26,7 +26,11 @@ export const UserInformation: React.FC<{}> = () => {
{profile.value == null ? (
<LoginLink />
) : (
profile.value.preferredUsername
profile.value.preferredUsername == null ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

one thing with typescript is need to check undefined at times.

@kumare3 kumare3 self-requested a review February 24, 2021 01:14
@EngHabu EngHabu changed the title Use name if preferredUsername is empty fix: Use name if preferredUsername is empty Feb 24, 2021
@EngHabu EngHabu merged commit 0cdd983 into master Feb 24, 2021
@flyte-bot
Copy link
Collaborator

🎉 This PR is included in version 0.19.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@anrusina anrusina deleted the username-fallback branch January 13, 2022 00:23
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.

3 participants