Check for `sub` claim, not `username`, when validating

[rhansen]

Not all IdPs provide a username or email claim in the UserInfo response.

This is a partial fix to #309. A more complete fix would:

• Use sub and iss for whitelisting instead of username. (Or at least add sub+iss filtering as a separate option, and add warnings to username whitelisting that explain its security flaws.)
• Extract iss from id_token and save it in structs.User and jwtmanager.VouchClaims.

[Firstyear]

@rhansen I think that there should also be a check here for preferred_username which is the defined claim in OIDC per:

https://openid.net/specs/openid-connect-basic-1_0.html#StandardClaims

sub should always be the "primary key" but preferred_username is the correct username claim to use, and then you should fall back to username if preferred_username was not presented.

[aaronpk]

From the openid spec about the preferred_username:

The RP MUST NOT rely upon this value being unique

This is meant for display purposes, but not meant to differentiate between accounts. This is not at all the same as sub

[Firstyear]

I ... mentioned that. sub is the primary key, and preferred username is for display.

[aaronpk]

Unless I'm horribly out of date, Vouch doesn't have the concept of a display username, so it should never use preferred_username

[bnfinet]

@aaronpk that's correct, VP does not utilize preferred_username. It can be passed downstream in a header but VP doesn't use it.

[mig5]

that's correct, VP does not utilize preferred_username. It can be passed downstream in a header but VP doesn't use it.

From my experience though, Vouch does however expect a generic OIDC provider to return 'Username' in order to evaluate things such as its username 'whitelist', which is equally as wrong. This is overridden for certain specific vendor OPs supported in Vouch, but not for the generic provider.

In my own OP, the claim is called something like foobarID (the user cannot change it or influence the value of it, so I willfully ignore the warning in the spec about relying on something like it, like a username). I had to change my OP to also send a claim called Username with the same value as the foobarID claim, before I could use things like the Username 'whitelist' in the Vouch config.

The best solution would be to allow specifying the claim param that is to be treated as the Username. As has been mentioned here, the technically-correct value per the spec is that of the 'sub'. But in practical terms, for things like 'whitelist' or other evaluations on a 'human friendly' name, it would be good to be able to configure it.

Apache's mod_authz_openid for example lets you do Require claim someclaim:somevalue as an equivalent of Vouch's user 'whitelist'.

[turboMaCk]

Hello. I would like to use vouch-proxy but seeing this open since Aug 12, 2020 is sort of blocking for me. Is there any plan on how to address the issue?