fizzy icon indicating copy to clipboard operation
fizzy copied to clipboard

Mobile: Native log in

Open olivaresf opened this issue 3 weeks ago • 5 comments

Adds JSON API support for the magic link authentication flow, enabling native mobile clients to authenticate. Instead of setting a cookie as an added security measure, we're sending back a pending_authentication_token that needs to be returned to the server when validating a code.

POST /session.json { email_address: "..." }

  • Returns { pending_authentication_token: "..." } (201)
  • Token expires in 10 minutes.
  • If a magic link cannot be sent, the web redirects to session_magic_link_path, so there's no leak regarding email presence. For that reason, this endpoint always returns a pending_authentication_token even if no email was sent.
  • If an invalid email is sent, we return :unprocessable_entity just like the web.

POST /session/magic_link.json { code: "...", pending_authentication_token: "..." }

  • Returns { session_token: "..." } (200)
  • Returns 401 if token missing, expired, invalid code, or email mismatch

olivaresf avatar Dec 04 '25 23:12 olivaresf

  • I would recommend extracting the rate_limit_exceeded_message string into a constant or locale file, as it's duplicated across both controllers and might need to be updated in multiple places.
  • The safe navigation operator usage in Sessions::SessionsController#create with identity&.send_magic_link could mask issues—consider whether silently failing to send a magic link when the identity doesn't exist is the intended behavior, or if you should handle this case more explicitly.
  • In respond_to_valid_code_from, creating an access token and rendering user data in the JSON response assumes the API client should receive sensitive information immediately after authentication. I would recommend documenting the security implications or reconsidering what data gets exposed in this endpoint.
  • The rate_limit_exceeded method is now duplicated between Sessions::MagicLinksController and SessionsController. I would recommend extracting this into a shared concern or module to reduce duplication.
  • In Sessions::SessionsController#create, the HTML format still calls serve_development_magic_link(magic_link) even when magic_link is nil (if identity doesn't exist). This could cause unexpected behavior—I would recommend adding a guard clause or handling the nil case explicitly.
  • The X-Magic-Link-Code header is being set in the JSON response only in development, which is fine for development workflows, but I would recommend documenting this behavior clearly to prevent accidental exposure in production.

jyroscoped avatar Dec 04 '25 23:12 jyroscoped

  • I would recommend extracting the rate_limit_exceeded_message string into a constant or locale file, as it's duplicated across both controllers and might need to be updated in multiple places.
  • The safe navigation operator usage in Sessions::SessionsController#create with identity&.send_magic_link could mask issues—consider whether silently failing to send a magic link when the identity doesn't exist is the intended behavior, or if you should handle this case more explicitly.
  • In respond_to_valid_code_from, creating an access token and rendering user data in the JSON response assumes the API client should receive sensitive information immediately after authentication. I would recommend documenting the security implications or reconsidering what data gets exposed in this endpoint.
  • The rate_limit_exceeded method is now duplicated between Sessions::MagicLinksController and SessionsController. I would recommend extracting this into a shared concern or module to reduce duplication.
  • In Sessions::SessionsController#create, the HTML format still calls serve_development_magic_link(magic_link) even when magic_link is nil (if identity doesn't exist). This could cause unexpected behavior—I would recommend adding a guard clause or handling the nil case explicitly.
  • The X-Magic-Link-Code header is being set in the JSON response only in development, which is fine for development workflows, but I would recommend documenting this behavior clearly to prevent accidental exposure in production.

jyroscoped avatar Dec 04 '25 23:12 jyroscoped

Thanks for your feedback! This is a WIP and isn't ready yet 😄

olivaresf avatar Dec 05 '25 00:12 olivaresf

@olivaresf seems like you don't have your email address added to Github so it doesn't show proper attribution on your commits. (After rebasing, they look like I made them, but that should fix itself after you add your email address)

monorkin avatar Dec 05 '25 11:12 monorkin

I pushed a few small changes and added comments to point them out.

monorkin avatar Dec 05 '25 11:12 monorkin