weave-gitops icon indicating copy to clipboard operation
weave-gitops copied to clipboard

OIDC: No distinction between a user's ID and their name

Open makkes opened this issue 2 years ago • 5 comments

Describe the bug

When logging in at Weave GitOps with Azure AD configured as OIDC provider, then the sub claim which is the recommended way to identify users in Azure AD is also used as the user's name:

screenshot-69ab

Environment

  • Weave-Gitops Version: 0.29.0
  • Flux Version: 2.0.1
  • Kubernetes version: 1.27.1

To Reproduce Steps to reproduce the behavior:

  1. Deploy Weave GitOps.
  2. Create a Secret oidc-auth in the Weave GitOps Namespace configured for Azure AD with the claimUsername set to sub.
  3. Log into Weave GitOps.
  4. Click on the upper-right icon to open the drop-down menu.

Expected behavior

The menu says "Hello, Max Jonas Werner"

Actual Behavior

The menu says "Hello, tRbZ..."

Additional Context (screenshots, logs, etc)

Weave GitOps should hit up the UserInfo endpoint and fetch the user's actual name. Alternatively it could see if it finds that information in the ID token but that's not a given depending on the OIDC provider's configuration.

makkes avatar Aug 10 '23 14:08 makkes

@makkes We previously used the Userinfo endpoint, but we had an issue opened because we were hammering the userinfo endpoint.

bigkevmcd avatar Aug 22 '23 03:08 bigkevmcd

But, I definitely think we should make this better.

https://openid.net/specs/openid-connect-core-1_0.html#rfc.section.5.1

We could allow you to optionally configure a "display name" claim (defaulting to "name" perhaps).

bigkevmcd avatar Aug 22 '23 08:08 bigkevmcd

@bigkevmcd, we run into the same issue. The proposed solution sounds great and flexible.

Cajga avatar Sep 19 '23 11:09 Cajga

@makkes - Do you have any documentation on how to wire flux to Azure AD (App Registrations etc)? I'm not sure what to put in the oidc-auth secret ( I have client id, client secret, issuer with tenant id, etc)

absolutemikex avatar Oct 04 '23 03:10 absolutemikex

@absolutemikex I've been working on a guide for this but haven't gotten to move it to a mergeable state. You can see the current draft version here.

makkes avatar Oct 04 '23 07:10 makkes