Page MenuHomePhabricator
Differential D5260

[keyserver] add `source` field to `LogInRequest`
ClosedPublic
Actions

Authored by kamil on Sep 29 2022, 3:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 25, 10:27 PM
Unknown Object (File)
Mon, Nov 25, 10:27 PM
Unknown Object (File)
Mon, Nov 25, 7:59 PM
Unknown Object (File)
Mon, Nov 25, 7:58 PM
Unknown Object (File)
Mon, Nov 25, 7:58 PM
Unknown Object (File)
Mon, Nov 11, 4:46 PM
Unknown Object (File)
Fri, Nov 8, 3:17 AM
Unknown Object (File)
Oct 25 2024, 8:16 PM
View All Files
Subscribers
• abosh
ashoat
atul

Details

Reviewers
tomek
atul
ashoat
Commits
rCOMMf551b131b6a5: [keyserver] add `source` field to `LogInRequest`
Summary

Add source param on the server LogInRequest type. (task)

Test Plan
  1. flow check
  2. Test login with different sources (from the native, web, on session recovery when cookie expired) and make sure provided source is correct.
  3. Make sure it works for older native clients (e.g. pass source as undefined)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil created this revision.Sep 29 2022, 3:52 AM
kamil held this revision as a draft.
Herald added subscribers: • abosh, atul, ashoat. · View Herald TranscriptSep 29 2022, 3:52 AM
Harbormaster completed remote builds in B12517: Diff 17178.Sep 29 2022, 4:04 AM
kamil published this revision for review.Sep 29 2022, 4:42 AM
kamil edited the summary of this revision. (Show Details)
kamil edited the test plan for this revision. (Show Details)
kamil added a reviewer: atul.
kamil added a parent revision: D5259: [web/native/lib] make `source` param required on log in.
ashoat requested changes to this revision.Sep 29 2022, 7:30 AM

This is a really bad idea. You're going to cause all existing native clients to crash when they attempt to log in.

This revision now requires changes to proceed.Sep 29 2022, 7:30 AM
kamil updated this revision to Diff 17194.Sep 29 2022, 8:03 AM
kamil edited the test plan for this revision. (Show Details)

leave sorce optional on keyserver

kamil retitled this revision from [keyserver] make `source` param required on log in to [keyserver] add `source` field to `LogInRequest`.Sep 29 2022, 8:07 AM
kamil edited the summary of this revision. (Show Details)
kamil edited the test plan for this revision. (Show Details)
kamil added a comment.Sep 29 2022, 8:10 AM
In D5260#155039, @ashoat wrote:

This is a really bad idea. You're going to cause all existing native clients to crash when they attempt to log in.

Yea, my mistake...

Updated diff to leave this param as maybe and make it resolve this comment: https://phab.comm.dev/D5259#inline-34610

Harbormaster completed remote builds in B12530: Diff 17194.Sep 29 2022, 8:14 AM
ashoat accepted this revision.Sep 29 2022, 9:14 AM
This revision is now accepted and ready to land.Sep 29 2022, 9:14 AM
kamil mentioned this in D5259: [web/native/lib] make `source` param required on log in.Sep 30 2022, 1:52 AM
Closed by commit rCOMMf551b131b6a5: [keyserver] add `source` field to `LogInRequest` (authored by kamil). · Explain WhyOct 3 2022, 9:03 AM
This revision was automatically updated to reflect the committed changes.
kamil added a commit: rCOMMf551b131b6a5: [keyserver] add `source` field to `LogInRequest`.

Revision Contents
Changeset List

PathSize
lib/
types/
1 line

Diff 17295

View Options

lib/types/account-types.js

Loading...