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

Remove confusing “You are already signed in.” flash message #13547

Merged

Conversation

ClearlyClaire
Copy link
Contributor

When attempting to access the log-in page while already logged in,
Devise's require_no_authentication kicks in and sets a flash
message “You are already signed in.”

In almost all cases, this also causes a redirect to /web, which
does not display or clear flash messages, thus leaving the message
to a potentially much later date, like for instance, accessing
/preferences several minutes after being redirected to /web.

@ClearlyClaire ClearlyClaire force-pushed the fixes/already-signed-in-flash-message branch from 7935484 to 1f8abb1 Compare April 26, 2020 10:01
When attempting to access the log-in page while already logged in,
Devise's `require_no_authentication` kicks in and sets a flash
message “You are already signed in.”

In almost all cases, this also causes a redirect to /web, which
does not display or clear flash messages, thus leaving the message
to a potentially much later date, like for instance, accessing
/preferences several minutes after being redirected to /web.
@ClearlyClaire ClearlyClaire force-pushed the fixes/already-signed-in-flash-message branch from 1f8abb1 to e127e5f Compare April 26, 2020 10:05
@@ -111,6 +111,13 @@ def prompt_for_two_factor(user)
render :two_factor
end

def require_no_authentication
super
Copy link
Member

Choose a reason for hiding this comment

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

What does the upstream method do? Does it make sense to super instead of just doing what it does without the thing we don't want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does some more low-level checks to see if a user is logged in, and, if that's the case, redirects to after_sign_in_path_for and sets the flash message.

I thought about the two following alternatives:

  • do that in after_sign_in_path_for instead, but doing anything effectful in there seems weird
  • rewrite require_no_authentication entirely, but I'm a bit uncomfortable with that as it is lower-level Devise/warden stuff

@Gargron Gargron merged commit 45202f7 into mastodon:master May 10, 2020
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.

2 participants