Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

user.data populated on a user.registration event isn't available in a populate access token lamda for the first created access token #1545

Closed
BugShooter opened this issue Jan 10, 2022 · 9 comments

Comments

@BugShooter
Copy link

user.data populated on a user.registration event isn't available in a populate access token lamda for the first created access token

Description

I try add userId from custom database to JWT access token.

  1. on a user.create event in webhook I create a user record in custom DB (let's say customUserId)
  2. on a user.registration event we already have saved user in FA's DB (because I use transactional webhooks)
    so I use FA's API in webhook to put customUserId in FA's user.data object successfuly
  3. I use a populate access token lamda to get customUserId from user.data object and put it to JWT access token,
    but this doesn't happen
  4. if I logout, clear session and login then the populate access token lamda to get customUserId from user.data object and put it to JWT access token successfuly

Affects versions

1.31.0

Steps to reproduce

Precondition:

  1. Create webhook to put customUserId (it can be random string) in user.data by FA's API on user.registration.create event
  2. Create a populate access token lambda to get user.data.customUserId if exist, and put it to JWT access token

Steps to reproduce the behavior:

  1. Go to 'create account form'
  2. Fill email & password for new account (user must not be exist in FA database)
  3. Get the access token from a frontend application
  4. See that the token dosn't contain the customUserId

Expected behavior

I expect that user.data.customId will be available in the populate access token lamba on time when the first access token is generating

Additional context

I suppose that events should happens transactionaly in this way:
1 webhook on user.create finished and FA user entity exist
2 webhook on user.registration.create patch user.data object
3 only when step 2 is finished FA generate new access token and the populate access token lambda put info from user.data to the access token

@mooreds mooreds added the bug Something isn't working label Jan 10, 2022
@robotdan robotdan added this to the 1.33.0 milestone Jan 24, 2022
@robotdan robotdan modified the milestones: 1.34.0, 1.35.0 Feb 18, 2022
@robotdan robotdan modified the milestones: 1.35.0, 1.36.0 Mar 8, 2022
@robotdan robotdan modified the milestones: 1.36.0, 1.37.0 Mar 21, 2022
@robotdan
Copy link
Member

robotdan commented Apr 6, 2022

@BugShooter if I understand the issue. you are doing this:

  1. Enable user.registration.create event
  2. Create a user & a registration
  3. When you receive the user.registration.create event you call back to FusionAuth to set a custom attribute named user.data. customUserId.
  4. The JWT that is returned on the Registration API does not contain the user.data.customUserId

If that is correct, this is working as designed. This event can be configured as transactional, this means that if your webhook fails to return a 200 status code, we do not create the user registration.

In order to accomplish that - this whole process takes place inside of a single transaction. So any changes made outside of this transaction are not visible within an open transaction that was started before any other changes were made.

You could use the user.registration.create.complete event which gets called outside of the transaction, but you'll still have the same problem due to the timing of when your webhoook executes.

The only way that I can think of to ensure you get add a custom attribute to the user data during the user create process is to make an HTTP request in the JWT populate lambda to retrieve the value and place it in the JWT. However this won't allow you to persist it in the JWT, so you'd still need to add it to the user eventually.

One example may be:

  1. Listen for user.registration.create.complete, on this event add the value to user.data by calling the FusionAuth User API.
  2. In the JWT populate lambda, if user.customUserId is not defined, make an HTTP request to retrieve it from your backend.
  3. Future invocations of the lambda will have the user.customUserId value set so you can skip the HTTP request.

HTTP requests are available in Lambdas beginning in 1.35.0 for Essentials and Enterprise licenses.
https://fusionauth.io/docs/v1/tech/release-notes

@robotdan robotdan added working as designed and removed bug Something isn't working labels Apr 6, 2022
@glen-84
Copy link

glen-84 commented Apr 14, 2022

This issue concerns me, as we had planned to do exactly the same thing as the OP.

Your steps diverge from the OP in two ways:

  1. It'd be the user.create event, not user.registration.create.
  2. The customUserId would be added in a JWT Populate lambda.

Doesn't FA "await" user.create before getting to registration, before getting to the Populate lambda?

(pseudocode)

transaction {
	await UserCreateHooks();
	
	await UserCreateRegistrationHooks();
	
	commit();
}

// User data should be available here?
await JwtPopulateLambda();

If this doesn't work, and it's not easy to change, I was wondering about the idea of webhooks being allowed to return data, that is then merged into the user/registration object before it is persisted?

POST /webhooks/user-create { ... }
Response: {user: {data: {customUserId: 123}}}

Not sure if it would help in this case. (it may be useful for other webhooks as well)

The only other option we would have is to store the FA user ID in our database, and use that to authenticate the user in our API, but I think it'd be preferable to use our own IDs.

Your proposed solution will not work for us, as we don't have an Essentials plan. I hope another solution can be found soon.

@mooreds
Copy link
Collaborator

mooreds commented Apr 15, 2022

@glen-84 There are no current plans to have the await happen in the form you suggest, though it is an interesting solution. I haven't looked at that code path, but I know there are a lot of moving parts in the login process.

Unfortunately, it may be that FusionAuth isn't a good fit for your needs. That's a bummer, but we'd rather be transparent about it and let you find it out during a POC rather than during implementation :) .

@glen-84
Copy link

glen-84 commented Apr 15, 2022

Unfortunately, it may be that FusionAuth isn't a good fit for your needs.

After spending a non-trivial amount of time evaluating Auth0, and the same with FusionAuth (including writing most of the refresh token/PKCE code that is not provided by the libraries), this is not an option for us (my boss would kill me 😝).

FusionAuth ticks a lot of boxes (you've done great work), it's just these few things so far (#185, #1545, #1660, #1674) that would make our integration much smoother.

For this particular issue, it seems like it'd be quite a common use case, and it's surprising to me that the Lambda doesn't have access to new user data when the webhooks are synchronous. Is the current flow documented anywhere?

A clean solution to this would be nice, but if it's not possible then we'll be forced to store the user's FusionAuth ID in our database.

@robotdan
Copy link
Member

I am still not sure I follow what is happening. Can you please provide a very specific set of ordered steps you are taking?

For example, here is what I think you are doing, but please confirm and clarify with a lot of detail if I am not correct.

  1. You have configured FusionAuth to use self-service registration
  2. A user [email protected] signs up using the self-service registration form.
  3. When the user submits the form, FusionAuth will create the user and registration for this new user.

In step 3, when the user clicks the "Submit" button on the self-service registration form, FusionAuth performs the following steps in this order:

  1. Create the user with an email [email protected].
    • Fire user.create if configured.
    • Fire user.create.complete if configured.
  2. Create the user registration
    • Fire user.registration.create if configured.
    • Fire user.create.registration.complete if configured.
  3. Create a JWT
    • Call the JWT populate lambda if configured.

In this workflow, you want the JWT Populate to include any modifications to the user or registration made during the user.create.complete or user.registration.create.complete webhooks?

@glen-84
Copy link

glen-84 commented Apr 21, 2022

In this workflow, you want the JWT Populate to include any modifications to the user or registration made during the user.create.complete or user.registration.create.complete webhooks?

We want modifications in the synchronous user.registration.create to be included in the JWT Populate lambda. The complete hooks are asynchronous if I understand correctly, so they couldn't be used for this purpose.

(I would personally prefer to use user.create to add additional data to the user [since user.registration.create can be called once for each application], but this would have to be supported by accepting a response to the webhook call.)

NB. I have not yet tested this flow, as I had assumed that the OP had done so, but I will do so now, just to confirm.

@mooreds
Copy link
Collaborator

mooreds commented Apr 21, 2022

@glen-84 using user.create or user.registration.create will definitely not work because the underlying database transaction won't be closed. For example, if you listen for user.create for user [email protected] and, in the webhook, try to modify the [email protected] user object, it will fail because the transaction hasn't committed so the wider FusionAuth system doesn't know about that user yet. This is to support the higher level 'transaction' nature of the webhooks (where webhooks can stop processing of a user creation and other types of events).

The complete events are not asynchronous, they are just fired after the database transaction has occurred (and therefore [email protected] is available to the FusionAuth APIs).

More on that here: https://fusionauth.io/docs/v1/tech/events-webhooks/writing-a-webhook#calling-fusionauth-apis-in-webhooks

The user.create.complete webhooks are relatively new (since 1.30.0) so it is possible this will work.

Please let us know if you test out the scenario.

@glen-84
Copy link

glen-84 commented Apr 21, 2022

Okay, so I just tested this myself (twice), using the original instructions (user.create + user.registration.create) ... and it works.

For the 2nd test, I added an artificial sleep of 2 seconds to both webhook EPs and it still worked.

As for the OP, and assuming that this is not a timing issue, I can only assume one or more of:

  1. This was a bug that was fixed after 1.31.0 (probably not).
  2. The OP accidentally used an async complete hook.
  3. The OP forgot to assign the lambda to the application.

using user.create or user.registration.create will definitely not work

As mentioned above, it does appear to work. Does the transaction wrap both user-create and user-registration-create, or are they separate transactions?

i.e. (pseudocode):

transaction {
    await UserCreateHooks();

    await UserRegistrationCreateHooks();

    commit(); // save user and registration.
}

await JwtPopulateLambda();

vs:

transaction {
    await UserCreateHooks();

    commit(); // save user.
}

transaction {
    await UserRegistrationCreateHooks();

    commit(); // save registration.
}

await JwtPopulateLambda();

If it's the latter, then the data would be available in UserRegistrationCreate (although a failure in the 2nd transaction could lead to a user being created without a registration, which is not ideal). I just confirmed that this is true, if the registration hook fails, you're left with a user without a registration.

The complete events are not asynchronous

I just meant that FA won't wait for them to complete and fail the transaction if they fail.

More on that here

That document literally says that this use case is supported 😄:

Review available events and determine if a subsequent event occurs in your workflow. For example, user.registration.create may occur after a user is created. At this point, the user will exist and can be modified. If an event happens repeatedly, make the modification idempotent. In this case, the field is available as soon as the other event fires.

The user.create.complete webhooks are relatively new (since 1.30.0) so it is possible this will work.

We'd want it to be transactional, otherwise a failure in the UserRegistrationCreate hook could leave a user without an ID in the remote system.


In summary:

  1. This seems to work, but it'd be nice to clarify the transaction behaviour around user/registration creation.
  2. It might be worth considering whether registration creation should be part of the same transaction as user creation, to prevent user creation without registration creation. Of course, this would break the use case of modifying a user inside the UserRegistrationCreate hook, but there are probably better solutions for that anyway (see below).
  3. All that said, it's still not an ideal flow, modifying user data in a UserRegistrationCreate hook (which itself may fail, currently). It requires state to be kept between the two hooks (f.e. storing the FA user ID in the external database), which then partially defeats the purpose of working with external user IDs, if you're going to store the FA user IDs anyway.
    • In my opinion, the best solution would be to allow certain webhooks to return data. For example, inside a UserCreate hook, you could create a user in the external system, and return that user ID in the POST response. FA would then merge that data into the user object before persisting it. Only one hook is required, and it's transactional. Is it worth creating a separate issue for this suggestion, or do you not like it?

For now, I might just continue storing and using the FA user IDs, since the 2-hook solution is not rock solid.

@mooreds
Copy link
Collaborator

mooreds commented Apr 21, 2022

Hmmm. Thanks for testing.

I am afraid I might have been imprecise. What I meant is that modifying the user object from within the user.create webhook won't work. Similarly, neither will modifying the registration object from within the user.registration webhook. But modifying the user from within the user.registration webhook should work fine (as the doc says).

They are in two separate transactions, I believe, because they are not always an atomic item.

It might be worth considering whether registration creation should be part of the same transaction as user creation, to prevent user creation without registration creation.

If you think it is worthwhile, please file a new issue suggesting it and we'll see what community support it garners.

In my opinion, the best solution would be to allow certain webhooks to return data.

This would be another great feature request. I can't really speak to whether we'd implement it, depends on community and customer feedback as well as engineering level of effort. We already allow webhooks to return a status code to indicate success or failure, so maybe a post processing step (including a lambda) would make sense. Please do file an issue.

For now, I might just continue storing and using the FA user IDs, since the 2-hook solution is not rock solid.

That makes sense. For anyone else on this issue, I think the 2-hook solution will continue to work but I understand it may have side-effects.

I feel like we can close this issue since you tested this (extensively!), so I'm going to do so.

If someone else encounters this issue, please re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants