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

Use correct message when active_for_authentication? fails #533

Closed
wants to merge 1 commit into from

Conversation

panzi
Copy link

@panzi panzi commented Feb 10, 2016

This way a user can overload User#active_for_authentication? and User#inactive_message and the correct error message will be returned by the sessions controller.

This way a user can overload User#active_for_authentication? and User#inactive_message and the correct error message will be returned by the sessions controller.
@booleanbetrayal
Copy link
Collaborator

Not sure what you mean by failure, but I think the correct way to deal with overrides is through custom locale definitions.

@panzi
Copy link
Author

panzi commented Feb 14, 2016

No that is not what I mean at all. There can be various reasons for why the account is inactive. It can be that the user has to confirm the email address, or (in our system) that the user account was deactivated by and admin for some reason.

The devise examples tell you, you can handle that by overloading User#active_for_authentication? and User#inactive_message. The first tells devise that the account is deactivated, the second gives the reason for that (in form of a symbol). However, these examples break with devise_token_auth because that ignores the User#inactive_message method and uses a hard-coded key instead. This patch fixes this issue.

It is not about localisation. With localisation I still only have one message (one per language).

@booleanbetrayal
Copy link
Collaborator

Gotcha. Re-opened. Can you please update tests then?

@panzi
Copy link
Author

panzi commented Feb 14, 2016

When I have time. I have a deadline next Friday and plans on Saturday. So maybe next Sunday, if nothing else comes up.

@endorama
Copy link

endorama commented Apr 18, 2016

Hello guys, I need this functionality in an application I'm working on, could I step in to help in updating tests?

@zachfeldman
Copy link
Contributor

Hi there @panzi and @endorama ,

In an effort to cleanup this project and prioritize a bit, we're marking pull requests that haven't had any activity in a while with a "close-in-7-days" label. If we don't hear from you in about a week, we'll be closing this pull request. Obviously feel free to re-open it at any time if it's the right time or this was done in error! If you do, please rebase it with the latest master and explain why it's still needed.

We really appreciate your contribution, we're just trying to make this project manageable again to move it forward.

Hope all is well.

@newfylox
Copy link

newfylox commented Dec 6, 2017

Why this PR is closed? I just found that Devise Token Auth doesn't override inactive_message and I thought it was fixed reading this.

@zachfeldman
Copy link
Contributor

@newfylox :

"In an effort to cleanup this project and prioritize a bit, we're marking pull requests that haven't had any activity in a while with a "close-in-7-days" label. If we don't hear from you in about a week, we'll be closing this pull request. Obviously feel free to re-open it at any time if it's the right time or this was done in error! If you do, please rebase it with the latest master and explain why it's still needed."

this is a volunteer-run project. If you have the time to fix up this PR, we're more than happy to review it now that the project is more active, but it did not have any activity since April of 2016 which is a year and a half ago.

@m4-miranda
Copy link
Contributor

@zachfeldman I'd be interested to open a PR towards this. But first I'd like to have something cleared up:

As mentioned here,

def inactive_message
    :inactive
end

the :inactive locale key from Devise is returned as the response.

However, this project defines it's own set of messages, as seen here,

def render_create_error_not_confirmed
    render_error(401, I18n.t("devise_token_auth.sessions.not_confirmed", email: @resource.email))
end

def render_create_error_account_locked
  render_error(401, I18n.t("devise.mailer.unlock_instructions.account_lock_msg"))
end

def render_create_error_bad_credentials
  render_error(401, I18n.t("devise_token_auth.sessions.bad_credentials"))
end

Why aren't the locales defined by Devise being used? I've found the devise_token_auth locales file and assume that this is done project-wide.

P.S: Ruby/Rails beginner, so totally possible that I'm missing something simple here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants