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

[AXON-46] Add remote auth flow using atlascode-backend #135

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

sdzh-atlassian
Copy link
Member

What is this?

The next PR in the chain to enable a smooth 3LO-from-anywhere experience in the extension ;)

With this, we can now use an external service to bounce authentication from - in this PR it's referred to as the JiraRemote OAuth strategy

Short recap of the changes:

  • Fork the existing monolithic OAuth process, split it in 2 parts (start via user action / finish via deeplink)
  • Add a button (completely invisible to everyone, FF pending) to trigger the new oauth
  • Add an ability to configure OAuth client from the environment on the fly, for dev purposes
  • Move OAuth strategy data into their own file, now with a bunch of docs
  • Implement URIHandler path for /auth
  • A whooooole lot of IPC/messaging boilerplate 😞

What's notably missing:

  • OAuth attempt/session management
  • The way to trigger new auth outside of dev environment is effectively hidden
  • PKCE for the new Jira auth
  • Bitbucket remote auth

There are also some known issues - for instance, the credentials seemingly get bundled between the strategies and somehow invalidate themselves - rather inconsistently, at that 😞

image

I'm leaving the follow-up to follow-up PRs. Also, some of the issues might be addressed by #79

How was this tested

  • Unit tests, especially the new one on Strategy/StrategyData to prevent regressions
  • E2E tested the new flow
  • E2E tested the old flow

cabella-dot
cabella-dot previously approved these changes Feb 20, 2025
Copy link
Contributor

@cabella-dot cabella-dot left a comment

Choose a reason for hiding this comment

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

overall looks good to me, just those 2 nits

marcomura
marcomura previously approved these changes Feb 20, 2025
Copy link
Contributor

@marcomura marcomura left a comment

Choose a reason for hiding this comment

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

two minor comments, LTGM

@sdzh-atlassian sdzh-atlassian merged commit 42974a1 into main Feb 20, 2025
2 checks passed
@sdzh-atlassian sdzh-atlassian deleted the AXON-46-wip branch February 20, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants