-
Notifications
You must be signed in to change notification settings - Fork 22
Create a verify-email page #16
Comments
I implemented my last suggestion and created a GraphQL Mutation The Mutation works similar to the I first thought about using a MongoDB Transaction here to set both Account and user, but realized there is probably no point in doing that - If the Account is not present, the user documents email can still be set to validated and would be copied over to the account when that is created later. Should I submit a PR for each identity and api, or does this need further discussion / a separate reaction-api issue? |
@janus-reith Thanks for creating this issue. I was aware of this being missing but the verification flow has been a bit broken and untested even in 2.x, and the stock API does not check verification anywhere. That said, it is something we want to support. We just need to do it correctly. Verify is similar to password reset in that it involves sending an email and the request initiates from an email link. I got password resets working, but I didn't get them 100% to where I wanted them. IMO currently too much is done by the API (namely, token generation). This means the API knows more about identity and users than it really should. I haven't had time to vet it, but my thought was that both reset and verify should actually have most code moved to reaction-identity. Only the email sending would need API. If that is possible, then we would just need to add a generic Alternatively, if figuring out the authorization is too complex for now, the existing verify/reset mutations could be kept but changed to expect token generation is already done and token and user data for the email is passed in. Hope this makes sense. If you have anything you want to PR in this direction, we can use that for further discussion. Otherwise this is currently on our post-3.0.0 project list. |
@aldeed The PR I meant to submit would just make use of the existing functionality in the api. That link just doesn't work currently, so basically what I did was:
Seems to be very stable so far. I think I get the point regarding the splitted concerns, but I'm not sure I can follow how this should be approached on short term: But sure, this can wait, it's just something that currently is broken in the beta release. Doing it this way seemed better than to strip out the welcome email functionality of the api. |
@janus-reith Did you try handling the verification entirely on identity? I've been trying to use only vanilla Meteor Accounts for identity. So in theory the identity page can just call Accounts.verifyEmail with the token. In the callback, it should also call The rest of what I was talking about was in regards to initiating a validation flow after the initial welcome. |
@aldeed I skipped using Accounts.verifyEmail altogether due to the considerations in the initial post:
The mutation sitting in the API makes sure that the email is set to verified not only in users, but also in Accounts and works entirely with the token from the email. No signin in Identity should happen during the process. |
I was afraid that might be true. I still think it would be best to use the Meteor API, and then call the API after that only for purposes of synchronizing the The main goal is to have zero references to the |
And I'll also look into whether tracking email addresses and their verification status on Account is even necessary. Ideally maybe that would even be a separate concept entirely. So rather than just "what is your email address?", the question would be "what email address(es) should receive notifications?", etc. For Reaction standpoint, we should only care about the latter type of question, and we may be even less concerned about verifying it (though support for verifying it is useful). |
@aldeed Yes, that sounds reasonable so far. I like the more abstracted approach on this, but I’m not sure yet where to get started. The part that probably really needs to be dealt with first is, how the Login Provider/Meteor and the API could communicate without both accessing the same DB Data and without unnecessary added complexity. Maybe that solution could take a more general approach that could be used for other services aswell that might be separated at some point? Also, if GraphQL is the right fit for this, or if there might be something more barebone or more complex (More in the direction of some Messaqe Queue Maybe? Kafka although versatile would probably be a bit too complex to include in the general project setup) |
@janus-reith Kafka will most likely be assumed to be part of the stack soon, though we're not quite there yet. Where we're at now (as of today actually), is that only the "authentication" API plugin is aware of and accesses the "users" collection. So by replacing that plugin you could link up a different identity provider. But it still works by both accessing the same DB, which as you say maybe isn't ideal. Possibilities:
And in the other direction, Identity really only needs the API for sending emails and perhaps to sync email verification status, though there may be other ways to achieve that. I'll be continuing to work through some of this prior to the final 3.0 release. Appreciate your ideas. |
@janus-reith Would you mind sharing your implementation as I don't see a PR for this? This is still not addressed by the RC team after a year so your implementation will be the best there is at this time since you mention it works and is stable! Thanks. |
When creating the new user Account, the API will create a Meteor-compatible secret as verification Token and stores it in the the users collection, similar to the default Meteor Accounts Behavior. It then sends a welcome Email, which has the default env REACTION_IDENTITY_PUBLIC_VERIFY_EMAIL_URL set to http://localhost:4100/#/verify-email/TOKEN.
That Route doesn't exists, create it. And maybe change the /#/ to /accounts/ like the other routes. Need to decide how to use this: The default Meteor way of calling Accounts.verifyEmail still works, but would only set the email to verified in the users collection.
Maybe the better option would be to not use the native Accounts.verifyEmail and instead call a GraphQL Mutation, like done for the password Reset. That could ensure both users and Account entries are set correctly. Also makes sense, as the token Generation also happens in the API already.
The text was updated successfully, but these errors were encountered: