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

using clearance without actionmailer? #957

Closed
gingerlime opened this issue Jan 26, 2022 · 10 comments
Closed

using clearance without actionmailer? #957

gingerlime opened this issue Jan 26, 2022 · 10 comments

Comments

@gingerlime
Copy link
Contributor

I'm trying to update our Rails app to remove actionmailer, since we don't use it, but I'm still getting an error about ActionMailer constant missing, which seems to come from Clearance...

Is there any way to prevent the clearance mailer from loading?

@dorianmariecom
Copy link
Contributor

I guess you would have to override Clearance::PasswordsController#deliver_email for password changes?

@gingerlime
Copy link
Contributor Author

Thanks @dorianmariefr. That's not really the problem, because I'm using our own PasswordsController anyway. However when the app is set with config.eager_load = true (usually in staging/live), then I get this error because it's trying to eager load ClearanceMailer, which inherits from ActionMailer, which isn't defined if my app no longer uses require "action_mailer/railtie" in config/application.rb.

This is the error I get with eager_load:

/usr/local/bundle/gems/clearance-2.5.0/app/mailers/clearance_mailer.rb:1:in `<top (required)>': uninitialized constant ActionMailer (NameError)

@dorianmariecom
Copy link
Contributor

Maybe you can remove clearance's app/mailers from your load path?

@gingerlime
Copy link
Contributor Author

I'm not sure I understand what you mean?

@dorianmariecom
Copy link
Contributor

In my opinion we do need action mailer, not sure what would replace it (sending password resets by text? by slack? other channels?). I think it's reasonable to expect users to have ActionMailer loaded

@dorianmariecom dorianmariecom closed this as not planned Won't fix, can't repro, duplicate, stale Jun 12, 2022
@gingerlime
Copy link
Contributor Author

In our case, we're using an API to send emails, so we're not going via ActionMailer.

It is reasonable, I can't say that it isn't. But it would be nice(r) if it wasn't causing an error if an app uses Clearance and doesn't use ActionMailer. Or if you could at least suggest a workaround?

@dorianmariecom
Copy link
Contributor

To be honest I'm not sure, I think you are better forking the gem, removing "actionmailer" from the gemspec, removing the "app/mailers/clearance_mailer.rb" file and keeping your passwords_controller.rb

@gingerlime
Copy link
Contributor Author

Forking is always a possibility, but I treat it as a last-resort. I've already contributed to Clearance and would be potentially happy to contribute more if there's a will on both sides. By the way, I also reported a potential security issue that wasn't even replied to.

@dorianmariecom
Copy link
Contributor

Feel free to submit a pull request and we can discuss solutions.

For security issues it's better to email [email protected] / https://thoughtbot.com/security

I'm going through the backlog of issues (already did the pull requests which I prioritize) but it's going to be mostly on Fridays and maybe weekends in my spare time.

@gingerlime
Copy link
Contributor Author

gingerlime commented Jun 13, 2022

I'll keep the email in mind for future. Is it documented anywhere on the repo? I don't remember if I saw it when reporting the issue. However, it's been there for quite a long while with not as much as a comment. EDIT: see #971

As far as submitting PRs. I already did (to solve another security issue) and generally happy to, but before I put work into it, I would ideally like to understand that the project is willing to consider such improvement/change beforehand. Otherwise it can be quite frustrating to both sides. If there's willingness to make ActionMailer optional, I can try to think of some changes and contribute them.

Thanks for maintaining the library. We definitely appreciate seeing some activity here :) and I'm personally happy to contribute to it as well.

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

No branches or pull requests

2 participants