What's the difference between setting the issuer this way and using the azure tenant flag?

What's the difference between setting the issuer this way and using the azure tenant flag?

using azure tenant flag doesn't set the issuer as azure provider never verifies the token until oidc issuer is set

Would it be feasible for oauth2-proxy to calculate the issuer url from the tenant ID? Then you'd have to specify it only once, in --tenant=MYTENANT and not twice additionally with --oidc-issuer-url=.../MYTENANT. (Not to detract from this docs change, just thinking out loud.)

@weinong Ok that makes sense, could you please copy the change to the versioned documentation as well please?

@wosc I think that would probably make sense, though at the moment, we are going through some major changes with the providers and trying to reduce the amount of code we are maintaining within them. I'd prefer we keep a note of this and implement the functionality post all of this refactoring so we don't make things even more complicated while we are working on this

Note the suggestion in this PR is for v1 tokens.

If the azure provider is updated for v2 endpoints, for v2 tokens the issuer changes to https://login.microsoftonline.com/{tenant-id}/v2.0 to match that of https://login.microsoftonline.com/{tenant-id}/discovery/v2.0/keys which can be found from https://login.microsoftonline.com/{tenant-id}/v2.0/.well-known/openid-configuration

Note that v2 tokens aren't the default, even when using the v2 endpoint for oauth2. To produce v2 tokens, in the Azure Portal, the app registration needs to be updated: its Manifest's accessTokenAcceptedVersion needs to be set to "2" in addition to using the v2 endpoint. (The v2 endpoint can send either 1 or 2 tokens, and 1 is often the default, I think?)

If I have to set an OIDC issuer URL, I fail to see how the "azure" provider is better than just using v2 tokens and the oidc provider instead of the azure provider? It's confusingly documented, but it sounds like just setting the OIDC Issuer URL is enough to start using the oidc provider?

Would it be feasible for oauth2-proxy to calculate the issuer url from the tenant ID? Then you'd have to specify it only once, in --tenant=MYTENANT and not twice additionally with --oidc-issuer-url=.../MYTENANT. (Not to detract from this docs change, just thinking out loud.)

since --tenant is optional so I would probably not want to derive from it.

@JoelSpeed, updated versioned doc.

@LouisStAmour unfortunately, azure provider uses adal which requires v1 issuer format. v2 issuer format works only with generic oidc provider.

What does ADAL offer that the generic provider lacks? From what I’ve tested both v1 and v2 work great with the generic oidc. It’s not clear to me why I would want to use a custom provider? :)

Especially if issuer isn’t being inferred, etc. And if all the provider does is let me not have to infer a URL, well, we could save a lot of code just by re-using the generic oidc instead…

@LouisStAmour if you want to get access token for an AAD resource, oidc will not be able to satisfy. Also, due to the lack of v2 support in the golang sdk, obtaining access token supports v1 endpoint only.

@weinong Interesting. Looks like there’s already talk of expanding the generic oidc code to handle this use case: #1140 Microsoft’s implementation appears to be standard, and documented at https://docs.microsoft.com/en-us/azure/active-directory-b2c/openid-connect#get-a-token so again, I suspect we would be able to switch to a generic oidc provider once a config setting exists for response_code in the generic provider, etc.

Personally I would have more faith in the security and implementation of OAuth2-proxy if there was one code flow that could handle all (standard) endpoints without needing to pick different provider-specific implementations. I recognize that provider-specific implementations can have additional security measures or conveniences but it would seem to me that a standard set of functionality is better tested and supported than trying to keep up with bug fixes or third-party dependencies in well, third-party dependencies?

@LouisStAmour interesting read! looking forward to it!

@JoelSpeed is there anything else you want me to change?

@JoelSpeed looking for feedback. thanks!

I think this is fine to merge for now, but long term, we will be replacing the Azure provider with a V2 compatible version based on the OIDC provider, so we will fix up the tenant vs oidc issuer discrepancy later