-
Notifications
You must be signed in to change notification settings - Fork 659
OCPBUGS-56892: Console can only show user name instead of full name as the display name #15522
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
base: main
Are you sure you want to change the base?
OCPBUGS-56892: Console can only show user name instead of full name as the display name #15522
Conversation
…hook in telemetry and masthead components
… data retrieval and dispatch behavior
@Leo6Leo: This pull request references Jira Issue OCPBUGS-56892, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/jira refresh |
@Leo6Leo: This pull request references Jira Issue OCPBUGS-56892, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
…th fallbacks and add corresponding unit tests
@Leo6Leo: This pull request references Jira Issue OCPBUGS-56892, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
const userToggle = ( | ||
<span className="co-username" data-test="username"> | ||
{authEnabledFlag ? username : t('public~Auth disabled')} | ||
{authEnabledFlag ? displayName || username || 'User' : t('public~Auth disabled')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
displayName
will always return a value so i am not sure if we need the fallback values here
return currentUsername.trim(); | ||
} | ||
// Final fallback for edge cases | ||
return 'Unknown User'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should either add i18n or return null (to allow for fallback values to fallback)
frontend/packages/console-dynamic-plugin-sdk/src/app/redux-types.ts
Outdated
Show resolved
Hide resolved
frontend/packages/console-dynamic-plugin-sdk/src/app/core/reducers/coreSelectors.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: logonoff <[email protected]>
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a comment regarding capitalization
frontend/packages/console-shared/src/hooks/__tests__/useUser.spec.ts
Outdated
Show resolved
Hide resolved
frontend/packages/console-shared/src/hooks/__tests__/useUser.spec.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: logonoff <[email protected]>
Co-authored-by: logonoff <[email protected]>
/retest |
1 similar comment
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
Does this work / have to work with impersonation?
/cc @TheRealJon
I think you should also take a look as you're more familiar with the redux store
I don't think this will any impact with the impersonation, since the user will still be authenticated as the user they are logged in. Impersonation will show a banner at the top of the console, and it won't change the identify of the current logged in user. |
/retest |
frontend/packages/console-dynamic-plugin-sdk/src/app/core/actions/core.ts
Outdated
Show resolved
Hide resolved
…ons/core.ts Co-authored-by: logonoff <[email protected]>
|
||
type GetImpersonate = (state: SDKStoreState) => ImpersonateKind; | ||
type GetUser = (state: SDKStoreState) => UserInfo; | ||
type GetUserResource = (state: SDKStoreState) => any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more spot where we can remove the use of any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks for catching that @logonoff
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Leo6Leo, logonoff The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest verify by |
@Leo6Leo: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Closes #13744
Description
After logging in with Google OAuth OIDC, the Console can only show username instead of full name as the display name on the top right corner in the console.
Step to test out the changes
Have a cluster that configure the Google OIDC login. If you don't have one, you can follow the instructions I wrote in the Appendix which locates at the bottom of this PR description.
Choose
Google-OpenID-Connect
as your login option and login with your google accountSpecial Note
Thanks to @logonoff for the discussion on what would be the better way to resolve this issue.
Appendix: How to configure Google OAuth on your cluster for testing purpose
Prerequisites
Before you begin, you'll need:
Step 1: Create the Google Client Secret in OpenShift
First, create a k8s secret in the openshift-config namespace to securely store your Google OAuth client secret. This prevents storing sensitive values directly in the main cluster configuration.
Replace
<your-google-client-secret>
with the actual client secret from your Google Cloud project.Step 2: Configure the Cluster OAuth Identity Provider
Next, edit the cluster-wide OAuth configuration to add Google as an identity provider.
Replace with the Client ID from your Google Cloud project.
Save and close the editor. Then wait until the authentication operator finish reconciling.
Step 3: Get the redirect_uri and Update Google Cloud
This is a crucial step. OpenShift generates a unique callback URL that Google needs to know about for security.
Open your OpenShift cluster's web console in a browser. You should now see the "Google" login option.
Click the Google login button. You will likely be redirected to a Google error page saying something like "Error 400: redirect_uri_mismatch". This is expected!
Copy the entire URL from your browser's address bar. Find the redirect_uri parameter within that URL. It will look something like this:
https://oauth-openshift.apps../oauth2callback/Google
Go to your project in the Google Cloud Console.
Navigate to your OAuth 2.0 Client ID.
Under the "Authorized redirect URIs" section, click "ADD URI" and paste the full redirect_uri you copied.
Click Save.
Login with your google account again and you will see it works!