chirpstack-application-server icon indicating copy to clipboard operation
chirpstack-application-server copied to clipboard

Optional email_verified field in oidc implement

Open JaMurphSmi opened this issue 3 years ago • 2 comments

Summary

The email_verified field should be made an optional component in the oidc validation process, where a success condition can still be obtained without it.

What is the use-case?

My organization does not have infrastructure in place to send the email_verified field as it has to be a generalized system to work with multiple implementations of SSO. As such, the provided fields have been reduced to only the mandatory for several implementations. It would be great if optional fields in oidc docs were implemented in optional way.

Implementation description

The email_verified field can either be removed from the condition entirely, or there could be an aspect of the conf, where perhaps a value user-supplied in the configuration can act as a default.

JaMurphSmi avatar Aug 30 '21 11:08 JaMurphSmi

I have this problem too, and it is preventing me from deploying ChirpStack using Microsoft Azure AD.

Currently, GetUser (in oidc.go) makes a UserInfo call and takes the claim from there rather than using the claim from the token.

Microsoft documents that it is not possible to alter the information returned in UserInfo, and that any customisation (such as optional claims) should be done by reading the token instead.

Therefore the current implementation means that it is impossible to supply "email_verified" using Microsoft Azure AD.

It might be possible to get this into the token, however Microsoft also documents that all email addresses are verified so applications shouldn't really need to check this field.

While I appreciate that other use cases with other identity providers may well require this check, it's necessary to make this optional in order to interoperate with Microsoft Azure AD.

It might also be worthwhile making the choice between calling userinfo vs taking the claim directly from the token a configuration option too.

gavanfantom avatar Jun 14 '22 14:06 gavanfantom

I have this problem too, and it is preventing me from deploying ChirpStack using Microsoft Azure AD.

Currently, GetUser (in oidc.go) makes a UserInfo call and takes the claim from there rather than using the claim from the token.

Microsoft documents that it is not possible to alter the information returned in UserInfo, and that any customisation (such as optional claims) should be done by reading the token instead.

Therefore the current implementation means that it is impossible to supply "email_verified" using Microsoft Azure AD.

It might be possible to get this into the token, however Microsoft also documents that all email addresses are verified so applications shouldn't really need to check this field.

While I appreciate that other use cases with other identity providers may well require this check, it's necessary to make this optional in order to interoperate with Microsoft Azure AD.

It might also be worthwhile making the choice between calling userinfo vs taking the claim directly from the token a configuration option too.

Thank you for this, your suggestions fully align with my own points.

This is the exact same issue I have been having. I have also tried making the case on the Chirpstack Application forum to have the UserInfo dependency removed, or at least made optional as swapping one dependency for another (for the exact inverse reason that we need the idToken from Azure) just causes more chaos.

That code change has unfortunately steamrolled us and made an SSO implementation impossible with the current version.

JaMurphSmi avatar Jun 17 '22 08:06 JaMurphSmi

Pull request #695 contains two new options which implement this. With these changes I am able to authenticate against Azure AD.

gavanfantom avatar Aug 10 '22 12:08 gavanfantom