ADFS: relying party identifier configuration

[lcgkm]

I found vouch-proxy use RedirectURL as relying party identifier by default. Could we configure the relying party identifier in config.yml by ourselves?

func setDefaultsADFS() {
	log.Info("configuring ADFS OAuth")
	OAuthopts = oauth2.SetAuthURLParam("resource", GenOAuth.RedirectURL) // Needed or all claims won't be included
}

[bnfinet]

for context..

URIs as object identifiers: relying party identifier https://docs.microsoft.com/en-us/windows-server/identity/ad-fs/technical-reference/how-uris-are-used-in-ad-fs

If the RPI is a URI is it okay to have it be https://vouch.yourdomain.com/auth?

Is that a problem for you?

[lcgkm]

However, the relying party identifier is usually a UUID, not a URI.

[lcgkm]

Is that a problem for you?

RedirectURL is not equal relying party identifier. It's hard to talk with our ADFS admin. They don't allow us add RedirectURL as relying party identifier.

[bnfinet]

@simongottschlag what do you think of this suggestion?

[simongottschlag]

@bnfinet @lcgkm I haven't had the problem since we usually manage ADFS ourselves. I don't see it as a problem making it configurable - I just hadn't thought about it when implementing it.

Maybe make sure to use redirectUri if none is defined in the configuration, to keep backward compatibility?

[bnfinet]

@lcgkm I know this is an old thread but I wanted to clarify that I'd be open to a PR

[tangel0v]

Maybe I can open an PR to you with this @bnfinet as I have been stuck in my other issue and if I fix this one, I'll fix my problem too.

In the last versions of Microsoft ADFS you can pass the Relaying Party Trust as part of the scopes of OAuth (for example: "$MY_PRECIOUS_APP/openid allatclaims" as scopes will set openid and allatclaims as scopes and $MY_PRECIOUS_APP as Relaying Party Trust ID.

Do you think is better to add one variable in the configuration for Relaying Party Trust ID in ADFS provider?

[bnfinet]

@tangel0v are you saying #257 is related to this issue?

Thanks for the offer of a PR, that's always appreciated. However...

In the last versions of Microsoft ADFS you can pass the Relaying Party Trust as part of the scopes of OAuth (for example: "$MY_PRECIOUS_APP/openid allatclaims" as scopes will set openid and allatclaims as scopes and $MY_PRECIOUS_APP as Relaying Party Trust ID.

If ADFS sets the RPI to $MY_PRECIOUS_APP when oauth.scopes includes $MY_PRECIOUS_APP/openid would it be better to just document this? Would that be sufficient to satisfy the feature request? That'd be better than adding a new knob to Vouch Proxy IMHO.

[tangel0v]

Not exactly. I was facing this issue in my configuration therefore I tried to use the standard oidc.

Putting the Redirect URL as Relying Party Trust ID just works (in ADFS versions 2016 and 2019 at least), but it brokes all our RP IDs naming convention: that's the reason why I was trying to use oidc.

About this:

If ADFS sets the RPI to $MY_PRECIOUS_APP when oauth.scopes includes $MY_PRECIOUS_APP/openid would it be better to just document this? Would that be sufficient to satisfy the feature request? That'd be better than adding a new knob to Vouch Proxy IMHO.

I am using this feature with Grafana and it does well but I was not able to get it working with Vouch Proxy. I don't see anything weird :S. I think it's better to add the configuration as older ADFS version doesn't support it.

[bnfinet]

@tangel0v makes sense, a PR would be welcome

[mario-tux]

@tangel0v Could you explain in details if there is a way to make ADFS works with VP without PR but with a configuration of ADFS where the RPI is not the RedirectURL? We can't change the ADFS configuration (different management). It looks that patching the code of VP to not set the resource field (remarking the setting in setDefaultsADFS function), as in a oidc-like configuration, it just works.

@bnfinet If you are available we can provide a PR that, on request, can pass a specific RPI in a adfs configuration.

@Herbrant

[mario-tux]

We submitted a PR (https://github.com/vouch/vouch-proxy/pull/393) to cover the OP support request: it should guarantee backward compatibility. It works in our application scenario with a UUID as relying_party_id. Please, consider its mainstream inclusion.

@Herbrant