Developers could reuse user ids and unknowingly violate the OIDC spec

[mooreds]

User id can be reused, violates OIDC spec

Description

Per section 8 of the OIDC spec, the sub claim should be both unique in the tenant and never re-used.

A Subject Identifier is a locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client.

https://openid.net/specs/openid-connect-core-1_0.html

But you can reuse a userid in FusionAuth, which is used as the sub claim. Doh!

Affects versions

1.24

Steps to reproduce

1. Create a user using the admin screen. Capture the assigned user id in a text file.
2. Delete the user completely.
3. Create a new user in the same tenant with the curl script:

FAUID=[previously used user id]

curl -H "X-FusionAuth-TenantId: $TENANT_ID" -XPOST -H "Content-type: application/json" -vv -H "Authorization: $APIKEY" https://auth.example.com/api/user/$FAUID -d '{ "user": { "email" :  "test3@example.com", "password" : "password" }}'

This user is created and a token is returned showing the old user id as the sub claim.

Expected behavior

FusionAuth should fail with a 400 status code and a generic error message. I would suggest not revealing that this user id had been used before in the error message (so as not to reveal that info to an attacker), but perhaps an event log would make sense? I could see this issue being confusing, so some kind of server side logging makes sense.

[robotdan]

Hmm, not sure I agree. While per spec, perhaps, but there is a lot of things you can do out of spec if you want.

This would require the user of this API to be assigning the user id during user creation. If you provide your own Id, I think you as an app dev are now taking responsibility to make sure unique ids are not reassigned to a new user. FusionAuth would have no way to know if this id represents the same user as previously defined in FusionAuth or a new user. This decision would have to be left up to the application developer.

This is working as designed as far as I can tell. If we enforced this type of behavior, if you accidentally deleted a user, you would have no way to create the user again if you needed to provide the Id which may correspond to an external system.

[mooreds]

Well, you could implement this by having a table that just has previously used user ids (populated any time a user is deleted), and check that on user creation whenever an ID was supplied. We wouldn't have to change the API to always force a user id to be provided (unless I am missing something). Would definitely want to document this behavior.

I agree this is an edge case and we can choose not to implement, though.

But it seems clear we're out of spec. Might be an issue if we ever pursued #359 (don't know how tightly they exercise all aspects of the spec).

[robotdan]

Yeah, you could argue it is not to spec.

However this is a user API, not an OIDC API. We simply provide building blocks or tools to implement these things.

We generate UUIDs, which are unique, once you provide your own id, you have now taken responsibility for this behavior.

We could leave this open as a future possibility, but it would be very low priority.

[mooreds]

Can't argue with the priority :) !

As far as I can tell, the sub claim in the id_token is the FusionAuth user id, so this user api behavior bleeds over into our OIDC implementation. You can't modify that in a populate lambda, either.

Anyway, happy to let it be for now.

[voidmain]

I'm not sure that this is breaking compliance with the spec. Our user API is acting more like a conduit to the database than an enforcement of any particular protocol or specification. While a stretch, you could then say that PostgreSQL is not spec compliant because they allow id reuse. It is up to the developer to ensure they are compliant or not. I don't think this is the job of our User API.

Additionally, there might be other specifications that don't have this constraint and therefore could conflict with OIDC is this manner.

Also, user ids have a wide-range of applications and implications in FusionAuth currently and that is going to be expanding when we implement #1 and other features. Since developer ergonomics is a core principle of the platform, we don't want to make it more challenging for developers to try and reason through reasons why API calls start failing or users aren't able to login. Specifically, we want to avoid issues like: https://stackoverflow.com/questions/46529363/specify-user-id-when-creating-user-in-keycloak. We often encounter these types of situations and our approach is always to allow the maximum of flexibility without compromising security.

Just considering how Connectors work, this change could cause issues for anyone leveraging a Connector. If the Connector isn't migrating users, this could have even more issues.

My opinion is to document this rather than change how FusionAuth handles creating users and managing user ids.

[mooreds]

I don't want to beat a dead horse, but this is how I arrived at my message:

• we implement the OIDC spec
• the OIDC spec says the sub claim can never be reused
• the user id in FusionAuth maps directly to the sub claim in the id_token and there's no way to unmap that currently
• the user id can be reused. Not a good idea, but could happen.

That all said: I agree with @robotdan that this is very low priority. And with @voidmain that this could cause other issues; I didn't even consider connectors. And I think that documentation is a good idea, perhaps if/when we try for OIDC certification. It may be that the OIDC cert tests wouldn't discover this because it's such an edge case and involves proprietary API calls.

[robotdan]

If we document it, I would want to be very clear that “you” as an api user can reuse ids, and if “you” do that, in theory if “you” delete and recreate users “you” could be out of spec in that “you” may end up reusing a “sub” claim for two users who are not the same user IRL.

Just to be clear that it is not anything FusionAuth is doing, but we do allow for this scenario.

Any API worth its weight will let you do things you perhaps ought not do. 😆

[robotdan]

Closing as working as designed.