# Discourse doesn't re-verify an address changed by SSO
**URL:** https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887
**Category:** Bug
**Tags:** sso, discourseconnect
**Created:** [8 juli 2015 om 09:52 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887 "2015-07-08T09:52:08Z")
**Posts on this page:** 20
**Page:** 1
### Author:  [@fefrei](https://meta.discourse.org/u/fefrei)
#### Post date: [8 juli 2015 om 09:52 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/1 "2015-07-08T09:52:08Z")
If a SSO provider sets `require_activation=true`, users must confirm their E-Mail address after they first sign in with SSO.
When a user changes his E-Mail address in a non-SSO setting, he must verify his address before the change is saved.
When the setting `sso overrides email` is enabled, and a user changes this E-Mail address in the external system, his address is changed in Discourse without verification, even with `require_activation=true`. I think that this is inconsistent. I’d expect that in this setting, the account is disabled whenever the address changes, requiring the user to verify his new address.
---
### Author:  [@downey](https://meta.discourse.org/u/downey)
#### Post date: [8 juli 2015 om 13:14 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/2 "2015-07-08T13:14:15Z")
~~This seems like a horrible idea for our use case.~~
~~Discourse trusts our SSO provider (user directory) enough to authenticate users, so why shouldn’t it also trust that system to validate users’ email addresses?~~
~~Our users would become extremely annoyed if they had to validate their email addresses twice on each change.~~
OK, maybe I misunderstood. I just hope this is not the default behavior.
---
### Author:  [@fefrei](https://meta.discourse.org/u/fefrei)
#### Post date: [8 juli 2015 om 13:20 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/3 "2015-07-08T13:20:13Z")
Correct, this is not about the default behavior. Normally, Discourse trusts the E-Mail address provided. If the SSO payload sets `require_activation=true` (which shouldn’t be the case for you), it doesn’t trust the address for signups (which is intended), but trusts it for address changes (which this report is about).
---
### Author:  [@sam](https://meta.discourse.org/u/sam)
#### Post date: [8 juli 2015 om 13:22 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/4 "2015-07-08T13:22:13Z")
> [@downey](#):
>
> I just hope this is not the default behavior.
you got to opt in with `require_activation=true` it’s a legit edge case of a bug
---
### Author:  [@simon](https://meta.discourse.org/u/simon)
#### Post date: [16 augustus 2016 om 23:40 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/5 "2016-08-16T23:40:02Z")
I’m running into this when `sso overrides email` is enabled on Discourse and a user changes their email address on a WordPress site. How big of a security risk is this?
Adding `user.active = false` to `DiscourseSingleSignOn#change_external_attributes_and_override` for the case when the user’s email doesn’t match the email sent with the sso request forces the user to confirm their new email address. Unfortunately it does this by redirecting them to `users/account_created`, so it’s a bit of a hacky solution.
Edit: the problem is that WordPress will let a user change their email without verifying it. The wp-discourse plugin attempts to verify the email on registration, and if it can’t it sets `require_activation` to true. Possibly the simplest solution is to recommend that sites using the plugin should not enable `sso overrides email`.
---
### Author:  [@fefrei](https://meta.discourse.org/u/fefrei)
#### Post date: [17 augustus 2016 om 18:18 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/6 "2016-08-17T18:18:35Z")
> [@simon](#):
>
> How big of a security risk is this?
I haven’t tested this, but…
- If there is an “unclaimed” developer email address in `app.yml` (rather unlikely to happen), this could allow any user to gain administrative rights.
- It allows spamming, because you can change your address to an address you want to spam.
- It _might_ allow taking over a staged account if changing the email address merges the accounts, but I’d assume it doesn’t (and errors out instead).
I don’t see any other risk right now, but user identity is strongly tied to the email address in Discourse, so it’s hard to estimate the impact.
---
### Author:  [@simon](https://meta.discourse.org/u/simon)
#### Post date: [17 augustus 2016 om 22:45 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/7 "2016-08-17T22:45:43Z")
A proper fix for this seems like it could be time consuming. Would it be possible to change the setting’s description to include a warning to not enable it unless the email address is verified by the SSO provider? Or even more specifically, to not enable it with the wp-discourse plugin?
---
### Author:  [@codinghorror](https://meta.discourse.org/u/codinghorror)
#### Post date: [17 augustus 2016 om 22:55 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/8 "2016-08-17T22:55:17Z")
Absolutely, it should have a huge warning, with details.
---
### Author:  [@sam](https://meta.discourse.org/u/sam)
#### Post date: [17 augustus 2016 om 23:19 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/9 "2016-08-17T23:19:11Z")
Sure this is a bug we should fix. I need to add some tests for it.
---
### Author:  [@fefrei](https://meta.discourse.org/u/fefrei)
#### Post date: [9 februari 2017 om 20:12 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/10 "2017-02-09T20:12:45Z")
Just a small update, because I was asked via PM:
The bug is still open in the latest beta release, `v1.8.0.beta4 +284` (`9fa29ca89815878f768885f7413b8beaae3a414c`).
---
### Author:  [@codinghorror](https://meta.discourse.org/u/codinghorror)
#### Post date: [9 februari 2017 om 21:25 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/11 "2017-02-09T21:25:23Z")
Maybe @sam should add some tests for it?
---
### Author:  [@rysher](https://meta.discourse.org/u/rysher)
#### Post date: [25 april 2017 om 18:44 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/13 "2017-04-25T18:44:01Z")
Did @sam add some tests for it? It seems that this issue still exists.
---
### Author:  [@sam](https://meta.discourse.org/u/sam)
#### Post date: [25 april 2017 om 22:26 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/14 "2017-04-25T22:26:04Z")
It is on my list, I have not addressed anything here yet.
---
### Author:  [@sascha](https://meta.discourse.org/u/sascha)
#### Post date: [28 april 2017 om 01:28 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/15 "2017-04-28T01:28:58Z")
I’d like to vote for this as well. Our IdP currently allows users to change their email address without verification, which in 2 steps allows a user to completely hijack an existing Discourse user.
1. Change the email address to some other existing user, sign on using SSO, it will update the external\_id of that user.
2. Change the email address back, now the external\_email of that user is updated as well.
Does it make sense to provide a way of disabling 1)? The external\_id of a user should be immutable and the email address should not be used to look up users?
And of course, updating a users email address in Discourse with require\_activation=true should send an activation email for that address.
---
### Author:  [@DeanMarkTaylor](https://meta.discourse.org/u/DeanMarkTaylor)
#### Post date: [28 april 2017 om 02:00 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/16 "2017-04-28T02:00:44Z")
> [@sascha](#):
>
> Change the email address to some other existing user, sign on using SSO, it will update the external\_id of that user.
> [@sascha](#):
>
> Change the email address back, now the external\_email of that user is updated as well.
This goes against the requirements set out in the SSO topic:
> [@Setup DiscourseConnect - Official Single-Sign-On for Discourse (sso)](https://meta.discourse.org/t/setup-discourseconnect-official-single-sign-on-for-discourse-sso/13045/1):
>
> external\_id is any string unique to the user **that will never change** , even if their email, name, etc change. The suggested value is your database’s ‘id’ row number.
---
### Author:  [@sascha](https://meta.discourse.org/u/sascha)
#### Post date: [28 april 2017 om 02:58 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/17 "2017-04-28T02:58:31Z")
> [@Setup DiscourseConnect - Official Single-Sign-On for Discourse (sso)](https://meta.discourse.org/t/setup-discourseconnect-official-single-sign-on-for-discourse-sso/13045/1):
>
> Discourse will validate that the nonce is valid, and if valid, it will expire it right away so it can not be used again. Then, it will attempt to:
>
> 1. Log the user on by looking up an already associated external\_id in the SingleSignOnRecord model
> 2. Log the user on by using the email provided (updating external\_id)
If Discourse updates the external\_id of a user based on their email address, isn’t that violating the requirement itself that external\_id will never change? It’s effectively merging two separate external users into one discourse user if their email address is the same.
---
### Author:  [@fefrei](https://meta.discourse.org/u/fefrei)
#### Post date: [28 april 2017 om 09:07 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/18 "2017-04-28T09:07:19Z")
Did you verify that this actually happens? (By my amateurish understanding of the code, it does.)
If it does, I think this is a (separate) security-relevant #Contribute > Bug – while Discourse should match users by email if the external\_id is unknown, it should _not_ find a user that already has a _different_ external\_id and update it.
Maybe this code should back out with an error if `user` already has a (different) `external_id` set?
>
---
### Author:  [@sascha](https://meta.discourse.org/u/sascha)
#### Post date: [4 mei 2017 om 06:20 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/19 "2017-05-04T06:20:56Z")
This is actually what happens in my installation, the documentation / official SSO howto mentions this. I believe this is contradicting the aspect before that external\_id is immutable, if Discourse updates the external\_id, if an existing user is found by email address. While this could be desirable because it makes moving users between external systems easier, I see the security risk here as it allows an external system to both change the external\_id as well as the email address of existing users, completely disconnecting it from the original external system.
I don’t know the motivation behind this behavior though and do not know if failing in the case that the external\_id is already populated is desirable. After all the external fields are not editable by administrators in the front end and thus it would not be (easily) possibly to rectify a misconfiguration.
---
### Author:  [@rysher](https://meta.discourse.org/u/rysher)
#### Post date: [7 mei 2017 om 22:51 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/20 "2017-05-07T22:51:44Z")
It is definitely a critical security issue, if sso overwrites external\_id, it shouldn’t. But as I understand the code, it should set external\_id via match\_email\_or\_create\_user only if there have been no user with such external\_id found on sso login:
```
def lookup_or_create_user(ip_address=nil)
sso_record = SingleSignOnRecord.find_by(external_id: external_id)
if sso_record && (user = sso_record.user)
sso_record.last_payload = unsigned_payload
else
user = match_email_or_create_user(ip_address)
sso_record = user.single_sign_on_record
end
...
end
```
I’ll be able to check the behavior on my local setup tomorrow.
---
### Author:  [@rysher](https://meta.discourse.org/u/rysher)
#### Post date: [7 mei 2017 om 23:40 UTC](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887/21 "2017-05-07T23:40:58Z")
I’ve checked it and confirm that external\_id can be overwritten this way. And in combination with this topic’s issue an account can be hi-jacked this way. I agree with @fefrei that it should be added as a separate security-relevan #Contribute > Bug.
[Volgende pagina](https://meta.discourse.org/t/discourse-doesnt-re-verify-an-address-changed-by-sso/30887.md?page=2)