-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2023-06-28] [Secure Logins] Replace login uses with accountIDs throughout App (when accessing Onyx) #19007
Comments
Hi I'm Bartek from Callstack - expert contributor group - I would like to work on this issue. |
Amazing, thanks! Assigned 💪 Please let us know if you have any questions about what we're looking for here! |
Daily update: I've managed to cover around 60-70% of the |
Sounds good @burczu , that would be great to have this tomorrow 🙏 |
@burczu any updates here? |
@Beamanator please check below what I've got so far (I had no time to analyze helper methods yet, but I can do it on Monday). Please also let me know if you need me to fix this list in any way. List & categorize all changes needed to remove contact info from Onyx
List & categorize all changes needed to remove contact information from API calls
|
Looks like something related to As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our Feel free to drop a note in #expensify-open-source with any questions. |
Hey @burczu thanks for the effort so far! What you've got is definitely useful as a start! Here's a few more things I think we'd like to know:
Does that make sense? |
Hey @Beamanator! It does make sense! I'll continue working on it today. |
Daily update: I've made some progress yesterday, but I need more time, so I'll continue working on it. Hopefully I'll be able to post some updates to my report soon (today/tomorrow). |
Hey @Beamanator! Please see below the updated report. It should address this part of your last comment:
I've also covered all the helper methods that I've found are related to personal details. List & categorize all changes needed to remove contact info from Onyx
List & categorize all changes needed to remove contact information from API calls
|
@burczu thanks so much for the amazing updates to |
Hey @Beamanator! I've just updated my last comment by changing the It now contains what we had before, so the details about what we need to change in the Onyx data, as well as the information about the API calls parameters that may contain Now, I'll try to address this part form your comment:
But, because tomorrow we had internal workshop at Callstack, I'll probably be able to finish it on Monday... |
Daily update: still working on detecting places where contact info is updated in the NewDot app, a will continue tomorrow. |
Hey @burczu thanks for pushing forward! Just so you know, @puneetlath and I have been working hard on getting backend changes live so you SHOULD soon be able to start a PR for these changes! 🙏 We're starting with making sure You probably won't see any updates till late today or tomorrow, but we'll keep you posted! |
Hi @Beamanator, thanks for the update. I had to switch to another issue that is important for @puneetlath for a while, but will get back to this one probably today. |
Ok @burczu we are ready to start updating components. If you use NewDot with the staging server, you should see the new Now, we need to migrate every component that is currently using I think we would ideally break this up into many small PRs so that it is easy to review/test/catch regressions. I would imagine that we could have multiple devs work on this simultaneously. What do you think? |
FYI we're also planning to migrate Here's my investigation of the changes needed for this migration so far:
@burczu there may not be any additional migration to do for ^, but it would be helpful if you could validate my findings and let me know if you think I missed anything |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.36-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2023-07-12. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
Ok, I believe the last thing we need to do here is pay out all the C+ that helped out with this project. I will post in C+ to figure out who that is. Posted here: https://expensify.slack.com/archives/C02NK2DQWUX/p1690300828906829 |
I was the C+ for #22008 @puneetlath! |
@puneetlath |
I was C+ for #20035 |
Payment for four C+ for internal PR review:
I considered a half PR anything for which the full reviewer checklist wasn't done. Let me know if I missed anything or made any mistakes. @fedirjh @0xmiroslav I've sent you Upwork contract offers. @allroundexperts @Santhosh-Sellavel please request via NewDot. |
@puneetlath Sorry, can you please hold my payment until further notice? I am working on some stuff due to recent measurements in my region. And update issue to Monthly or create separate GH for me. Thanks |
Sounds good. @allroundexperts @Santhosh-Sellavel let me know when you've requested payments. |
I've requested the payment @puneetlath! |
Reviewed details for @allroundexperts. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot. |
Requested on ND |
@JmillsExpensify assigning you for @Santhosh-Sellavel's payment as well. |
Reviewed details for @Santhosh-Sellavel. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot. |
@puneetlath I am ready to get paid as discussed |
All payments made. Closing this out. Thanks again everyone for your help on this one! |
Part of "Secure Logins in EChat" project
As part of the "Secure Logins in EChat" project, we want it to be impossible to scrape contact information from NewDot. One of the biggest refactors we will do for this project is: Re-keying the
ONYXKEYS.PERSONAL_DETAILS
data to have accountIDs as keys (currently the keys are logins - emails or phone numbers). This will take a large effort on the server side as well as the front-end, so we need some help getting everything together on the front-end.In this issue, we will focus on two parts:
List & categorize all changes needed to remove contact info from Onyx
PERSONAL_DETAILS
onyx key.[login]
to[accountID]
OptionsListUtils.getNewChatOptions
ReportUtils.getIcons
report.managerEmail
toreport.managerAccountID
(this requires backend changes)List & categorize all changes needed to remove contact information from API calls
email: currentUserEmail
for each split participant, but we could useaccountID: currentUserAccountID
insteadassignee
param sends an email / login. If so, we can update that to be something likeassigneeAccountID
The text was updated successfully, but these errors were encountered: