skygear-server
skygear-server copied to clipboard
Include session in user create event
Developers may be interested in session properties (e.g. IP, user agent) in user creation web-hook event. We should include the newly created session in user create event, so that:
- If user is signed up normally:
payload.session
is the newly created session,context.session
is null. - If user is created manually:
payload.session
is null,context.session
is session of the request user.
If the session cannot be created immediately (e.g. MFA is required), both payload.session
and context.session
may be null.
@kiootic I think this issue is good, but can you also update the feature spec and also guides / API spec altogether? :)
(btw, to help ensure we don't miss things, it would be good if from now on we follow the cycle of "create feature issue => create implementation, docs issues => update implementation and cos => close feature issue" cycle)
Actually I've spent the morning think about it, and now I think we should drop this issue for now:
- There would be some sort of dependency between
user_create
andsession_create
events: the session cannot be created beforeuser_create
event, anduser_create
event cannot be generated without creating the session. - It would be tricky to implement hook handler correctly. Developer may assume
payload.session
is always non-null (as in common cases), but the assumption would be broken if we implemented some kind of API for user creation by admin. - It's near the time for release and it's better to freeze the features we had for now. It's not a breaking change to add it next version.
- It can be worked around by handling
session_create
event with session create reason = signup.
@louischan-oursky @carmenlau @chpapa what do you think about it?
By the way, I think it's okay to open these kind of issues here at first, since I'm not sure how the actual design would goes at first. I think the flow goes like:
- Developers open a feature/bug issue here.
- Maintainers discuss how to solve the feature/bug.
- If the change is major (e.g. requiring API/design change), open issue at feature repo.
- Cross-reference the feature issue in original issue.
Will look at it later, I haven't give much thought yet.
Just make sure if we have a change that should update spec and API references / Guides, we don't forgot the feature issue.
There would be some sort of dependency between user_create and session_create events: the session cannot be created before user_create event, and user_create event cannot be generated without creating the session.
Agree, it may not be good to have such dependency too
It can be worked around by handling session_create event with session create reason = signup.
Looks good to me
Other thought:
I think ip (and user agent) should not necessarily bind to the session, maybe we will want to have it in the context too? Developer may be interested to request ip before user login, e.g. detect user signup or login attempt in somewhere. We may not need to add this now, but it is good to have in future.