Skip to content
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

OAuth migration | Remove SC_GU_LA cookie check | Migrate MDAPI calls to use OAuth tokens #1294

Merged
merged 3 commits into from
Jan 18, 2024

Conversation

raphaelkabo
Copy link
Contributor

@raphaelkabo raphaelkabo commented Jan 17, 2024

What does this change?

This changes two things, one resulting from the other.

Remove the SC_GU_LA cookie check

After we enabled Okta on PROD MMA, we noticed a few browsers were experiencing redirect loops. We worked out that this was happening in the following situation:

  • The current Okta session was newer than the max_age parameter being sent by MMA.
  • Because of this, Okta considered the current session valid, and when a browser landed on MMA, immediately redirected the browser back to MMA, without refreshing or setting any cookies.
  • However, for some reason (unknown), the SC_GU_LA cookie was missing from the browser context in the affected browsers. I suspect this was a race condition: the SC_GU_LA cookie had expired and been deleted before the OAuth session became invalid, despite the fact that both are set to 30 minutes.
  • Seeing no SC_GU_LA cookie, MMA sent the browser back to Okta.
  • Repeat ad infinitum.

The error may have been limited to developer machines with funky cookie setups, but we reverted the config change on PROD just in case.

Luckily, the solution is simple - we do not actually need to check for the SC_GU_LA cookie, as our downstream APIs don't use it and we're going to deprecate it as part of our migration to Okta anyway. Thank you to @coldlink for solving this!

The first commit removes the check for SC_GU_LA during login.

Migrating MDAPI calls to use OAuth tokens

MDAPI supports authentication via IDAPI cookies and OAuth tokens. When making calls to MDAPI without the SC_GU_LA cookie set, these naturally fail. The second two commits in this PR update MMA to make calls to MDAPI only with the new OAuth access token, sent in an Authorization header, rather than the IDAPI cookies. This allows MMA to work without the SC_GU_LA cookie.

Currently, all other APIs still send the legacy IDAPI cookies. MDAPI needs to be updated because it is the only Guardian API which calls the IDAPI auth/redirect endpoint to validate IDAPI cookies - and that endpoint expects a valid SC_GU_LA value.

Tests

  • Tested on CODE
  • Tested on PROD

@raphaelkabo raphaelkabo changed the title OAuth migration | Remove SC_GU_LA cookie check OAuth migration | Remove SC_GU_LA cookie check | Migrate MDAPI to use OAuth tokens Jan 18, 2024
@raphaelkabo raphaelkabo changed the title OAuth migration | Remove SC_GU_LA cookie check | Migrate MDAPI to use OAuth tokens OAuth migration | Remove SC_GU_LA cookie check | Migrate MDAPI calls to use OAuth tokens Jan 18, 2024
Copy link
Member

@kelvin-chappell kelvin-chappell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

@raphaelkabo raphaelkabo merged commit c696668 into main Jan 18, 2024
11 checks passed
@raphaelkabo raphaelkabo deleted the rk/fix-oauth-cookie-bug branch January 18, 2024 14:27
@prout-bot
Copy link
Collaborator

Seen on PROD (merged by @raphaelkabo 3 hours, 25 minutes and 3 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants