-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
[$375] [HOLD for payment 2024-08-01] [Wave Collect] [Xero] Add 2FA requirement in Authorization flow #43015
Comments
Triggered auto assignment to @sakluger ( |
|
Triggered auto assignment to Design team member for new feature review - @dannymcclain ( |
I'll add a few mockups when I have edit access to Figma again. |
@francoisl let me know if you need me to mock anything up or take care of anything in Figma! 🤝 |
Thanks. I got an email from Figma saying that Jon gave me edit access but I still can't seem to edit anything. Basically I was trying to edit the Initial Setup part of the Xero mockups and add the 2FA flow in between. I was thinking of making two variations:
|
Super weird. I see that you were upgraded yesterday, but when I went into the file it still said you were requesting access (I gave it to you). I wonder if that file just hadn't been refreshed or something since Jon upgraded you. Anyways, try it again if you want, or I'm happy to do some jammin'. |
Added a little flow here in Figma. I think it's better to tell people "Hey you need to enable 2fa to use xero" so I like including the intermediary modal. |
That's great, thanks Danny! Looks like I was wrong about the sign-in flow and I changed the text description, but we can also make it more generic to accommodate the case when people sign in and they need to enable 2FA because they're on a domain group with that requirement. |
Ah I see, yeah that makes sense to me! |
Looks great to me |
Any updates here? |
yo yo yo sorry i lost track of this issue. can you please assign it to me? |
PR raised #44059 |
@francoisl everything tests well but we might have some hiccups on how 2FA works. For the flow: admin signs in and has a policy connected to Xero.
Expected: the app becomes usable again. Screen.Recording.2024-06-20.at.08.mp4 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.11-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 2024-08-01. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR adding this new feature has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@sakluger i think 500 would be great because it was hard to nail down the nitty gritty of this task. |
@c3024 could you please add regression test steps |
Regression test proposalTest 1:Pre-requisite:Have an account Steps:
Test 2:Pre-requisites:Have an account Steps:
|
Payment Summary
BugZero Checklist (@sakluger)
|
Can I get a payment summary on this issue? |
Sorry about that! I missed it because it wasn't automatically moved to Daily. I chatted with @francoisl via DM to figure out what was going on here. We won't penalize for regressions here because the app was already unusable if you were an admin on a Xero workspace prior to the first PR. Regarding price, we both thought that $500 was a bit high, but given the trickiness, we'll pay $375 - a bit more than the standard $250. |
Job added to Upwork: https://www.upwork.com/jobs/~01f129776196b2ac5f |
Current assignees @rushatgabhane and @c3024 are eligible for the External assigner, not assigning anyone new. |
Upwork job price has been updated to $375 |
@JmillsExpensify the payment summary is ready: #43015 (comment) |
$375 approved for @rushatgabhane |
I completed the payment to @c3024 via Upwork. |
Problem
In order to remain compliant with Xero's third-party app requirements, we need to force workspace admins to enable 2FA before they can use the connection.
Additional internal context
Solution
For new connections:
For existing connections:
cc @lakchote @zanyrenney
View all open jobs on GitHub
Issue Owner
Current Issue Owner: @Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: