Update authentication APIs to enable HybridApp auth refactor

[Julesssss]

Problem

I am working with @war-in and @staszekscp on a HybridApp authentication refactor.

They are making good progress but require some backend changes in order to complete the work.

Solution

Update backend APIs to match required onyx data:

we localized three more commands which need to return tryNewDot value

• SignInWithGoogle
• SignInWithApple
• SignInWithShortLivedAuthToken (for SAML) We can unify SignInWithShortLivedAuthToken and SinginUser by moving value returned by SignInUser to BeginSignIn. We can also simplify the frontend code and return nvp_tryNewDot in the onyxData (like we do for e.g. credentials in BeginSignIn command) so those onyx updates would be handled automatically.

Onyx Data

{
  "onyxData": [
    {
      "onyxMethod": "merge",
      "key": "nvp_tryNewDot",
      "value": {
        "classicRedirect": {
          "dismissed": true/false
        }
      }
    }
  ],
  "jsonCode": 200,
  "requestID": "8ecc27b03b9abbcf-WAW"
}

[Julesssss]

This is lower than other issues in my HybridApp queue

[melvin-bot[bot]]

@Julesssss Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Julesssss]

Finishing up other HybridApp tasks, I should have time for this soon after the new year

[Julesssss]

Still waiting for the ability to prioritise

[Julesssss]

The front-end work is in progress.

[Julesssss]

@war-in asked me to provide these changes. I'll be picking up this issue soon.

[Julesssss]

THis is on my todo list after a fire, and one other HybridApp backend issue.

[Julesssss]

Aiming to work on this this week!

[Julesssss]

I have been held up by change in priorities, this is still on my roadmap after we ship HybridApp NewDot signup and redirect to classic tasks.

[Julesssss]

OKAY. I am not going to jynx myself this time...

@war-in, is the desired format under Onyx Data still correct? If so I'll start by refactoring the hardcoded tryNewDot response data into a valid onyx payload. I'll return the full NVP data this time.

We can also simplify the frontend code and return nvp_tryNewDot in the onyxData

I wanted to double check this because I recall there was a reason we needed this to be returned outside of an onyx update.

[war-in]

@Julesssss I think it's fine to return it in regular onyx update data but maybe let's keep it in both places for now? 🤔 We can remove it then once we confirm its fine

[Julesssss]

Sounds good to me 👍 I am a bit worried that during login when we build the onyxData the tryNewDot value hasn't yet been set and won't be available.

[Julesssss]

Okay, I'm returning the full NVP data in SigninUser locally:

Same for SignUpUser user:

[Julesssss]

Next week I need to figure out why tryNewDot is now always returning true. Auth?

[Julesssss]

Next week I need to figure out why tryNewDot is now always returning true.

Bug fixed. PR is ready for review, I just need to fix a remaining test case failure.

[war-in]

Glad to see progress here! I also updated OD and ND PRs with the newest main branches

So now those commands should return NVP, right?

• SignInUser
• SignUpUser
• SignInWithGoogle
• SignInWithApple
• SignInWithShortLivedAuthToken (for SAML)

As I mentioned in the issue description it would be nice to get nvp in BeginSignIn command (earlier we know about nvp the easier it gets in terms of handling the flow)

[Julesssss]

So now those commands should return NVP, right?

@war-in yeah, once merged the full tryNewDot NVP data will be returned within onyxData for all of these cases. In addition we'll return tryNewDotDismissed.

I'll deprecate tryNewDot as it is confusingly named, but for now I am returning that too:

[Julesssss]

To avoid duplicated onyx data I had to make a large refactor and update automated tests. That's all done now and the final step is to see if I can make the NVP retrieval more efficient.

[Julesssss]

@war-in could you made do without the returned tryNewDotRedirecDismissed param from SignInWithShortLivedAuthToken? I have the onyxData being returned but I would need to take extra time to make that available. I can do that later if it's necessary.

[Julesssss]

The changes are working well, but add some extra Auth calls for retrieving the NVP data. As this isn't ideal for performance I will raise an Auth PR to improve this. It'll delay this PR by a few days but will be worth the wait.

[Julesssss]

Finished up some improvements and test fixes. After dealing with a flakey test I have put the PR in initial review.

[Julesssss]

Hey @war-in, the following commands are now returning onyxData on staging and some will give you tryNewDotDismissed in the response:

• BeginSignIn
• SignInUser
• SignUpUser
• SignInWithGoogle
• SignInWithApple
• SignInWithShortLivedAuthToken (SAML / redirect from OldDot)

Let me know if you need anything else at this point. SignInWithShortLivedAuthToken, SignInWithGoogle, and SignInWithApple won't give you tryNewDotDismissed but I should be able to figure this out if you need it.

[melvin-bot[bot]]

@Julesssss Eep! 4 days overdue now. Issues have feelings too...

[Julesssss]

I'm going to close this issue. We can use the parent issue for the rest of the changes.

[Julesssss]

Reopening while the remaining Auth keys are refactored.

[melvin-bot[bot]]

@Julesssss Huh... This is 4 days overdue. Who can take care of this?

[Julesssss]

Auth PR updated, will ship for review if tests pass

[melvin-bot[bot]]

@Julesssss Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[Julesssss]

Auth PR deployed, closing