OIDC `preferred_username`

[ChrisPortman]

Hi

I'm using AzureAD as the OIDC provider, and for some reason, when processing the preferred_username claim, its returning an empty string. Would it be possible to expose the claim used for the username as a configurable.

Additionally, the preferred_username is listed as "mutable" and not recommended for use as a linking identifier (at least according to MS - https://learn.microsoft.com/en-us/entra/identity-platform/id-token-claims-reference). Previously when I've done OIDC stuff with MS, my user model had a Username and Displayname (Displayname defaulted to Username), and then when using OIDC, the username value would be sourced from the oid claim and the display name would be sourced from preferred_username/name/email (configurable probably).

The issue with the oid claim without the differentiation of username vs displayname is that the oid is a GUID which makes no sense in a UI.

Happy to help with dev, if you think this is an issue worth looking at. Happy to, as a first cut, just provide an MR making the username claim configurable (default to preferred_username). Later look at updating the user model to introduce the username vs displayname concept

[NHAS]

Hi again Chris, thanks for another excellent issue.

I think this may actually be two issues in one. First being "can we make preferred_username configurable" which is a definite yes, and a second security vulnerability in the fact I'm using preferred_username as an ID rather than a GUID in the claim.

I definitely agree that the first issue should be configurable and you're more than welcome to open a MR for that. As for the second, Im going to have to have a bit of a think about how to use the supplied GUID. If you want to raise that as an additional issue that'd be great!

[ChrisPortman]

I think for using the GUID (from the oid claim, it makes sense to have a web interface where users can auth in the first instance and that initial login creates the user account in the local data store. That account would look like something like:

{
  "username": "<uuid/guid>",
  "displayname": "<preferred username>"
  ...
}

Certainly a process that requires an admin to pre-create user accounts would be tricky, because locating the GUIDs are a pain. Once the users have performed an initial login, the admin can then perform other tasks like managing their group memberships (beyond what the groups claim may have) and register tokens etc.

[NHAS]

I've been meaning to make device registration SSO enabled for sometime actually. So it could be quite nice to have it that on first registration it creates the association.

Just makes it basically impossible to automate.

[ChrisPortman]

looks like in version 8, in the data.Config representation of config, there is the concept of having the username claim configurable: https://github.com/NHAS/wag/blob/85ab816d0c98b9c1320d7c9962d2b5cb23cb44fd/internal/data/config.go#L20

I just cant see anywhere its set, and there isnt a corresponding option used when loading from the config file

[NHAS]

Ah I am incredibly dumb and forgot that I'd implemented this:

This is in the oidc provider, you can configure it via the administrative UI under settings.

		deviceUsername := info.GetPreferredUsername()

		if len(o.details.DeviceUsernameClaim) != 0 {

			deviceUsernameClaim, ok := tokens.IDTokenClaims.GetClaim(o.details.DeviceUsernameClaim).(string)
			if !ok {
				log.Println("Error, Device Username Claim set but idP has not set attribute in users token")
				http.Error(w, "Server Error", http.StatusInternalServerError)
				return
			}

			deviceUsername = deviceUsernameClaim

		}

[ChrisPortman]

Yeah, I think the corresponding struct field just needs to be added here: https://github.com/NHAS/wag/blob/85ab816d0c98b9c1320d7c9962d2b5cb23cb44fd/internal/config/config.go#L104

[NHAS]

Ah yes, it does. I've just left it being configurable in the administrative UI rather than adding it to the json file. Apologies!

[NHAS]

I've just updated my comments to be less incorrect haha

[NHAS]

Finally getting back around to this. I'll be using the sub (or subject) data thats given in the user info as its immutable by spec.

[NHAS]

My solution to the security portion of this issue is just to store and check the Idp provided subject. So its probably no the best solution. But it is the one that makes this a security non-issue, and doesnt require significant reworking of users within wag.

[JSmith-Aura]

Hmm, seems this was somehow reverted and Im not sure how. I'll be adding it back today