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

[ENG-5170] Addon account reauthorization #2214

Merged

Conversation

futa-ikeda
Copy link
Contributor

@futa-ikeda futa-ikeda commented May 10, 2024

  • Ticket: ENG-5170
  • Feature flag: gravy_waffle

Purpose

  • Allow users to reauthorize an addon service account after the credentials have expired

Summary of Changes

  • Update account-setup-manager to handle reauthorization workflow
  • Update user-addon-manager and addon settings page to support reauthorization workflow
  • Some misc things:
    • Rename Provider.createAccountForNodeAddon to Provider.createAuthorizedAccount for clarity
    • Move AuthorizedAccount.configuringUser relationship to the subclasses as the inverse relationships vary based on service type
    • Allow creation of multiple configuredAddons for a given addon
    • Remove unused Provider.setNodeAddonCredentials

Screenshot(s)

  • Desktop
    image
    image
    image
  • Mobile
    image

image

Side Effects

QA Notes

@futa-ikeda futa-ikeda marked this pull request as draft May 10, 2024 18:54
@futa-ikeda futa-ikeda marked this pull request as ready for review May 14, 2024 20:05
Copy link
Contributor

@brianjgeiger brianjgeiger left a comment

Choose a reason for hiding this comment

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

A couple of questions, but nothing blocking.

<h3 local-class='page-heading'>
{{t 'addons.accountSelect.reconnect-account'}}
</h3>
<AddonsService::AccountSetupManager
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually a Manager or is it just an Account Setup component? I know this exists already, but I just noticed and thought it was a bit odd of a name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. This doesn't manage anything, so I can rename this guy to make it a bit more clear

mirage/views/addons.ts Show resolved Hide resolved
@futa-ikeda futa-ikeda merged commit b03be1b into CenterForOpenScience:feature/addon-services May 14, 2024
9 checks passed
@futa-ikeda futa-ikeda deleted the account-reauth branch May 14, 2024 21:40
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.

None yet

2 participants