New app with MFA #554
Draft
New app with MFA #554
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that we use the path_info to find a route, it may be enough just to move the ACTION to $c->url_for() and keep the rest in the ->query(), but I feel it's easier just to remove ->query and pass all the params to my_url.
For the time being, let's use $c->my_url() and tweak the result, but it might be better to use $c->url_for here as well.
my_url() may use a different action stored in the stash, but the menu links do not need such a trick.
As pause_2025 mounts on the /, not on the /pause, the links also need to be modified
pause_2025 just uses $c->url_for(), so we don't need to "fix" the action
Now that FixAction is removed, we should set the Action somewhere
It's easier just to use action names as paths, but I feed it better to prepend group names to make it clear which action belongs to which group. ->name() is added to help $c->url_for() return the correct path.
Almost all the login code is taken from ::Middleware::Auth::Basic::authentication. Instead of setting REMOTE_USER, we use Mojolicious's session to keep the user's id (for now; eventually it should be stored in the database).
We can't use mfa_secret32 to decide if the user uses MFA or not, because mfa_secret32 needs to be stored in the database as soon as the user first visit the MFA page; otherwise Auth::GoogleAuth generates a different secret32 when the user posts the authentication code (and thus the verification fails).
Right now we only use the Mojo's session, we just need to clear the session. Eventually we'll also need to remove a session table entry.
The secret should be the same for all the workers. It is used to encrypt Mojo's cookie(-based session).
…er (taken from a session) is_allowed_action is factored out from ConfigPerRequest::_set_allowed_actions
…ion does not have a user
user_secrets was used to be retrieved when a user is authenticated via Basic Auth, but we don't use it now. Maybe _retrieve_user is the best place to retrieve the secrets.
…m a session maybe because of session timeout
as it is usually used in an email or side menu
(until I release a new version of Mojolicious::Plugin::WithCSRFProtection)
as the current tests under action expects things work the same as app_2017. (We'll add action_2025 next)
… PAUSE::Web2025::App::Index
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a new pause_2025 application that allows you to log in from a login form (not by the good-old basic authentication). The new app mounts on /, and uses the path info to decide what to do (instead of the ACTION query parameter). If you use the new app, and enable multifactor authentication, you need to send another authentication code when you log in. If you need the current interface, the pause_2017 app is still available.
Not everything is ready yet; some links may be broken or point to the old interface; tests are not migrated; sessions are not stored in the database; not all the upload methods are tested; apply fixes suggested in #455; there may be more, but I hope you can get the picture.
Some of the things we need to discuss later: should we allow users who enable MFA to use the old interface (ie. basic auth)?; should we still use the pause99_ prefix? (well, it's 2025 now); can we replace ACTIONREQ with something like Mojo's $c->flash or something?