Skip to content

Conversation

@ardelato
Copy link
Collaborator

Description

This allows us to use iFixit for user creation and authentication.

CR Notes

A lot of the changes revolved refactoring some of the current logic with routing and authentication, but in the end we can easily support both authentication strategies without conditionalizing all of the frontend.

The authentication flow for iFixit will look like this for both registration and login:

  1. Click respective CTA button
  2. Get redirected to respective page on iFixit - i.e. /Join or /Login
  3. Get redirected back to the Restarters app with session cookie now set
  4. Use said session cookie to validate the session via the iFixit API
    This is where it differs based on the route

/register
5. Create a new user in the DB based on the user data retrieved from the iFixit API

  • We will also set the user to an Admin role in the Restarters app if they are an Admin on iFixit. However, this will only be applied once during the creation route. This way, their roles will not be consistently overwritten after creation.

/login
5. Updates some of the user data


I should note there is an issue with this change.

A current user cannot sign up/login with their iFixit account if they have already created an account that uses the same email address. For these users will most likely need to manually delete their accounts in order for them to recreate it through the new authentication strategy.

qa_req 0 I confirmed this worked on Cominor.

ardelato added 11 commits July 16, 2025 15:49
…ration

- Added CentralizedAuth middleware to manage authentication flows
- Introduced LocalAuthStrategy and iFixitAuthStrategy for different auth methods
- Updated .env and config files to include new auth settings
- Removed EnsureAPIToken and VerifyUserConsent middleware as they are now integrated
- Refactored routes to utilize centralizedAuth middleware for improved auth handling

This will allow us to easily configure and switch between different authentication methods.
We will actually be implmenenting the iFixitAuthService in later commits.
…cation

- Introduced iFixitAuthService to handle session validation against the iFixit API.
- Updated bootstrap/app.php to exclude 'session' in encrypted cookies.

We need to exclude 'session' from encrypted cookies because it's not
set by an external service. As such, we need an unencrypted cookie to
be able to communicate with the iFixit API.
- Updated User model to include external_user_id and external_username fields.
- Implemented syncFromExternal method to create or update users from iFixit data.
- Enhanced UserController's logout method to handle redirection for external users.
- Added getCurrentUser method in iFixitAuthService to retrieve user data from iFixit session.
- Improved iFixitAuthStrategy to authenticate users and log them in based on iFixit session data.

This mainly adds the ability to create/update users from iFixit session data.
This will also set the user's role to Administrator if they are an Admin
in iFixit. That said, it will only affect the user's role on first creation.
If we manually change the user's role, then it will not be overwritten by
their iFixit role.
…thentication

This refactors the CustomApiTokenAuth middleware to simplify
the authentication process and include the new auth strategies for setting
and retrieving the api cookie.
If there is already a valid session cookie, then we should redirect to the dashboard
page instead of viewing the landing page.
- Updated UserController to handle password recovery and reset for iFixit users.
- Added AuthStrategyManager methods for login, logout, and registration URL retrieval.
- Refactored LoginController to redirect iFixit users to external authentication.
- Improved iFixitAuthService to provide registration URL functionality.

These changes make it easier to support both authentication strategies
without requiring a bunch of conditionalization in the Frontend.
We still need to conditionalize some of the frontend components even after
addressing most of it with the routing.

These components revolve around changing a user's password and inviting
users to the platform.

We can't change the password for an externally created user so we need to
hide components related to that. In addition, we can't invite users to the
platform as they would need to create an account on iFixit first and
there is no current way to pre-register them.
- Updated chart version to 0.2.2.
- Added new authentication environment variables to the chart.
These values were added to individual envGroups but were removed from their
original location. Let's do a little boyscout to clean up the chart.
- Updated GroupController to send a simplified notification upon group approval.
- Enhanced GroupAddEdit component to ensure callback is only invoked when necessary,
  improving the handling of success and failure scenarios during group creation and editing.
mlahargou
mlahargou previously approved these changes Jul 17, 2025
Copy link
Member

@mlahargou mlahargou left a comment

Choose a reason for hiding this comment

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

CR 📱 Checks out to me!

@ardelato
Copy link
Collaborator Author

dev_block ⚡ There are some issues that I found when I deployed the new helm chart.

Similar to the Group Invite changes, we will only allow inviting users
if we are using local auth. We currently can't use SSO to invite users.
@ardelato
Copy link
Collaborator Author

ardelato commented Jul 17, 2025

un_dev_block ⚡ I fixed some issues where inviting users was still possible when we were using the external auth.

QA 👍 I confirmed SSO login works, auto approval still works as expected, and the Groups Management page/CSV import still works.

Copy link
Member

@mlahargou mlahargou left a comment

Choose a reason for hiding this comment

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

CR 📱

@ardelato ardelato merged commit 2ca4049 into hermes Jul 17, 2025
@ardelato ardelato deleted the feat--use-sso-login branch July 17, 2025 23:21
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