Skip to content

Conversation

@tadhgboyle
Copy link
Member

@tadhgboyle tadhgboyle commented Jun 9, 2025

Introduces middleware system. I've implemented it in a few areas which are loaded globally and on the frontend which do redirects based on user/site state (maintenance mode, force TFA, etc). This helps us remove some global procedural code as we try to get to a cleaner internal state.

@tadhgboyle tadhgboyle marked this pull request as ready for review June 9, 2025 02:51
return;
}

Redirect::to(URL::build('/maintenance'));
Copy link
Member

@partydragen partydragen Jun 9, 2025

Choose a reason for hiding this comment

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

Should not send users to maintenance page thats just annoying, And alot of users like just to refresh site to see if maintenance is complete

Also makes uses lose any details they might have in the URL

public array $exemptRoutes = [
'/user/connections',
'/oauth',
'/user/settings',
Copy link
Member

Choose a reason for hiding this comment

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

User should only be able to view /user/settings IF they are viewing enable_tfa

return;
}

// Skip if AJAX request, such as Alert or PM checks
Copy link
Member

Choose a reason for hiding this comment

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

Was well also to prevent the code from refreshing when user switch to next step as that refreshing the site

{
public array $exemptRoutes = [
'/logout',
'/user/settings' // Allow access to settings to enable TFA
Copy link
Member

Choose a reason for hiding this comment

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

Same here, ONLY allow enable_tfa section,

Now user details is exposed can can be modifyed and can even disable 2fa without using 2fa to login

@partydragen
Copy link
Member

Able to make modules add/modify exemptRoutes?

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