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

Cypht: Improve interface so user is warned about the need for App Password for Gmail-IMAP #538

Conversation

jeanpaulHamuli
Copy link
Contributor

@jeanpaulHamuli jeanpaulHamuli commented Nov 19, 2021

Improve interface so the user is warned about the need for App Password for Gmail-IMAP

Pullrequest

Issues

Checklist

  • None

How2Test

  • Settings->Servers
  • After adding an SMTP or an IMAP server then write in the username field an email address with a Gmail domain

Todo

  • None

@marclaporte
Copy link
Member

@jeanpaulHamuli Some have a Google-powered email that is not @gmail.com, so can you find a way to cover this use case?

@jeanpaulHamuli
Copy link
Contributor Author

jeanpaulHamuli commented Nov 22, 2021

Yes, I'm looking for other possible forms of Gmail address, then implementing a general solution.
Better if I find a google API to check if it's for them or not

@marclaporte
Copy link
Member

When users are adding an email via https://github.com/jasonmunro/cypht/tree/master/modules/nux we know which provider it is. And if the email is set up manually, we can compare IMAP/SMTP settings: https://github.com/jasonmunro/cypht/blob/master/modules/nux/services.php

@marclaporte marclaporte marked this pull request as draft November 27, 2021 18:38
@jeanpaulHamuli
Copy link
Contributor Author

jeanpaulHamuli commented Nov 27, 2021

Okay, I'm working on it @marclaporte

@jeanpaulHamuli jeanpaulHamuli force-pushed the improve_gmail_warning_msg branch 7 times, most recently from d5dc003 to 04a0026 Compare November 29, 2021 19:45
@jeanpaulHamuli jeanpaulHamuli marked this pull request as ready for review November 30, 2021 12:22
@jeanpaulHamuli jeanpaulHamuli force-pushed the improve_gmail_warning_msg branch 2 times, most recently from 4ee7cb9 to c68e949 Compare December 30, 2021 11:00
@jasonmunro
Copy link
Member

Appreciate the work on this, couple things:

  • One of the URLs has a long odd argument that looks like some sort of tracking id, likely should be removed
  • These notices need to be wrapped in ->trans() otherwise they can't be translated from English (note you have to deal with any embeded HTML like links by substituting them in after the trans() otherwise they are not long HTML)

Happy to add more details if you need guidance. Thanks again!

@jeanpaulHamuli
Copy link
Contributor Author

Thank you for your comment @jasonmunro,
I'm done with what you asked, only I'm not sure I fully understand this (note that you have to handle any embedded HTML like links by replacing them after the trans() otherwise they're not long HTML)

@jasonmunro
Copy link
Member

Thank you for your comment @jasonmunro, I'm done with what you asked, only I'm not sure I fully understand this (note that you have to handle any embedded HTML like links by replacing them after the trans() otherwise they're not long HTML)

sorry should have said "no longer HTML". The trans() call also applies htmlsafe, so any embeded html will end being rendered as text. I see you made multiple trans() calls as a way to work around that but the problem there is that is not easy to translate as it is bits of text that may not line up in other languages. The correct way to do this is to use %s placeholders and sprintf():

sprintf($mod->trans("long string to translate with a link here %s"), link)

@jeanpaulHamuli
Copy link
Contributor Author

Ok, I understand the idea, but the implementation is not too clear to me.
You mean like that taking my case?
sprintf($mod->trans("long string to translate with a link %s"), <a href="https://developers.google.com/gmail/imap/imap-smtp" target="_blank">here</a>)

@jasonmunro
Copy link
Member

Ok, I understand the idea, but the implementation is not too clear to me. You mean like that taking my case? sprintf($mod->trans("long string to translate with a link %s"), <a href="https://developers.google.com/gmail/imap/imap-smtp" target="_blank">here</a>)

Almost, I would go with this:

return '<div class="warning_message">'.
    sprintf($mod->trans('Make sure your %s less secure app access %s '.
    'and your App password are enabled. For more details about the '.
    'configuration of IMAP, POP and SMTP click %s here %s'),
    '<a href="https://myaccount.google.com/lesssecureapps" target="_blank">', '</a>',
    '<a href="https://developers.google.com/gmail/imap/imap-smtp" target="_blank">', '</a>').
    '</div>';
    ```

@jeanpaulHamuli jeanpaulHamuli force-pushed the improve_gmail_warning_msg branch 4 times, most recently from b096c8f to 439e2e9 Compare January 19, 2022 21:18
@jeanpaulHamuli
Copy link
Contributor Author

It's done.
Thanks @jasonmunro!

@jeanpaulHamuli
Copy link
Contributor Author

Is there anything else or a fix we need to add to this PR to make it more perfect to be merged?

@marclaporte
Copy link
Member

@jeanpaulHamuli GitHub says: "This branch cannot be rebased due to conflicts"

@jeanpaulHamuli
Copy link
Contributor Author

I don't understand why they say that, there is no conflict in this branch @marclaporte

@marclaporte
Copy link
Member

@jeanpaulHamuli ok, we'll see what @jasonmunro says

@marclaporte
Copy link
Member

Big picture about "On May 30, 2022, you may lose access to apps that are using less secure sign-in technology"
https://www.reddit.com/r/google/comments/t5q0f4/on_may_30_you_may_lose_access_to_apps_that_are/

@jeanpaulHamuli
Copy link
Contributor Author

The removing of the less secure app access feature puts this PR at a serious disadvantage

@dumblob
Copy link
Member

dumblob commented Jul 7, 2022

Disadvantage? You mean this PR becomes kind of void as the less secure access does not work any more, right?

@jeanpaulHamuli
Copy link
Contributor Author

Yes, but since we still have Gmail in the app, we can improve and warn the user that they cannot use gmail account in less secure app

@dumblob
Copy link
Member

dumblob commented Jul 7, 2022

Oh yeah, that is true. But I do not see any disadvantage there - it is as it is (because Google mandates how things will be).

@marclaporte
Copy link
Member

I have a Google for domains account, and it still works. But one of my Gmail accounts stopped working and I discovered this issue: #573

@marclaporte marclaporte marked this pull request as draft August 20, 2022 17:54
@marclaporte
Copy link
Member

@adrienmaloba Please revisit this, evaluate and make a recommendation.

@marclaporte
Copy link
Member

We are going to put this (and all Cypht-Gmail related work) on hold for a few months, as we focus on the release of Cypht 1.4 and improving Cypht for standard IMAP and SMTP.

This being said, I welcome anyone in the community to contribute code, bug reports or documentation with respect to Gmail and Cypht. There are a lot of Gmail accounts out there.

I use Cypht for a Gmail account (via IMAP) and it works fine. The only issue is that I need to sort by "Arrival Date" instead of "Sent Date".

@marclaporte
Copy link
Member

You mean this PR becomes kind of void as the less secure access does not work any more, right?

Indeed. Closing this.

Let's focus on "Review the whole process/code/documentation for Cypht to work for Google and Microsoft's email offerings" here: #776

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.

Authentication Failed
4 participants