Add external_id to token response

[zacharyfleck]

The default response does not provide a unique ID to match on. Discourse emails / usernames are subject to change, so matching on those values can result in duplicate accounts created in external systems.

This one line change provides the discourse User ID in the token response. This flow allows our system to catch the user ID, and do an upsert to the user in our database regardless of what username, email, or other value they have set.

[theSuess]

Thanks for the contribution, this is something which is bothering me as well (and should ideally be mapped to the sub field) but I don't think external_id is the correct value to use. From what I've gathered, this value is only populated when discourse itself uses an external auth provider.

It's been a while since I worked on this - is the user_id field passed through by discourse? If yes, we should use that

[zacharyfleck]

Actually in my testing, the external_id seems to be the value returned to represent the user's Discourse user ID. See an example response of available fields here:

admin=true
avatar_url=https://cdn.thefirepanel.com/original/2X/1/178365ae68bff6df581fb167525f04cf14e49e16.png
card_background_url=https://cdn.thefirepanel.com/original/2X/5/556bfa8dff107a709e58b27e5938d7bc3054d45d.jpeg
[email protected]
external_id=1
groups=trust_level_2,admins,staff,trust_level_0,trust_level_4,moderators,trust_level_1,techs,trust_level_3
moderator=true
name=Zach
nonce=<removed>
profile_background_url=https://cdn.thefirepanel.com/original/2X/6/6312c55312716577515f5cb7e1b20aa01972978f.jTheAlmightyZach
peg
return_sso_url=https://oidc.thefirepanel.com/oauth2/callback
username=TheAlmightyZach

When you mention sub do you mean in place of the existing subject, or as the name in the mapping?

[theSuess]

I just tested it, and you are right - I must have been working on outdated information.

To comply with the OpenID connect spec, we should ideally use the external_id as the sub in the claims.

Can you update your PR to set the subject to external_id and add the preferred_username to the discourse username?

[zacharyfleck]

Happy to make the change, but currently traveling internationally and won't return until the 1st. Once I'm home I can push those changes up.

[zacharyfleck]

Just to confirm before I make this change, as this would be a breaking change for most.. Is this what you're thinking? (with nicer formatting of course)

Claims: &jwt.IDTokenClaims{
			Issuer:      aroot,
			Subject:     values.Get("external_id"),
			Audience:    []string{},
			ExpiresAt:   time.Now().Add(time.Hour * 6),
			IssuedAt:    time.Now(),
			RequestedAt: time.Now(),
			AuthTime:    time.Now(),
			Extra: map[string]interface{}{
				"email":          values.Get("email"),
				"email_verified": true,
				"picture":        values.Get("avatar_url"),
				"name":           values.Get("name"),
				"groups":         strings.Split(values.Get("groups"), ","),
				"preferred_username":    values.Get("username"),
			},
		},

[theSuess]

Exactly - that would be it! I wouldn't think too much about breaking changes. We don't have a latest tag for containers (the one that builds from master is called master explicitly to call out that this is not stable) & if someone decides to update, it's on them to read the changelog

[zacharyfleck]

Changes pushed!